Opened 12 years ago

Closed 11 years ago

#16963 closed Cleanup/optimization (wontfix)

relocate base View class to views.base from views.generic.base

Reported by: Preston Holmes Owned by: Preston Holmes
Component: Generic views Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Much thought went into the technique used for exposing a python class to the URLconf and making it sane in the context of the View contract in a thread safe way.

However, the base class, View, is located in the generic module, implying that it is somehow tied to generic views.

I propose that this base class is a good default for anyone doing class-based views in Django, and that its use outside the context of generic views should be made explicitly obvious by relocating the location of the code outside of the generic views module.

Attachments (2)

relocate_view_class.diff (10.6 KB ) - added by Preston Holmes 12 years ago.
relocate_view_class-2.diff (13.8 KB ) - added by Preston Holmes 12 years ago.

Download all attachments as: .zip

Change History (10)

by Preston Holmes, 12 years ago

Attachment: relocate_view_class.diff added

comment:1 by Russell Keith-Magee, 12 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Makes sense to me, and the patch looks good, except for docs. There is a reference to the base view in the CBV reference docs, which needs to be updated.

by Preston Holmes, 12 years ago

Attachment: relocate_view_class-2.diff added

comment:2 by Preston Holmes, 12 years ago

Patch needs improvement: unset

Updated to fix docs (a couple errant refs to django.db.views also fixed)

Hopefully #16807 will further improve the docs, but the latest patch should at least be complete

comment:3 by Bas Peschier, 12 years ago

To be honest, all classes in views/generic/base.py feel like they could live one package up. These classes and mixins are all ... uhm... basic for CBV and not specifically tied to generic views.

in reply to:  3 comment:4 by Preston Holmes, 12 years ago

Replying to bpeschier:

To be honest, all classes in views/generic/base.py feel like they could live one package up. These classes and mixins are all ... uhm... basic for CBV and not specifically tied to generic views.

I think part of the issue is that the history of "generic" views were that they were only viable for the most basic of the generic cases. The current generic views are more of a toolkit, and so will be far more flexible and useful. So the word generic is perhaps no longer the best descriptor.

However I think that leaving these "toolkit" pieces in a submodule of views makes sense, while moving the fundamental implementation of exposing a class as a view-thread safe instance is worthy of being the base views module.

only the View class deals with the fundamental interaction of python classes with the Django request/response flow and infrastructure - while the other classes are just "one way of doing it" even thought those in base.py are pretty hard to argue with.

comment:5 by Preston Holmes, 12 years ago

Owner: changed from nobody to Preston Holmes
Status: newassigned

comment:6 by Aymeric Augustin, 12 years ago

I understand the proposal, but in my opinion, the advantages of moving this code to a slightly more appropriate location aren't really worth :

  • either the deprecation path and backwards-incompatibility;
  • or keeping backwards-compatible imports forever, so that there are effectively two ways to import these classes.

So -0 from me.

comment:7 by Tim Graham, 11 years ago

Patch needs improvement: set

No longer applies cleanly.

comment:8 by Preston Holmes, 11 years ago

Resolution: wontfix
Status: assignedclosed

I now think this should just be in a list of things we would have done slightly different about CBVs

Note: See TracTickets for help on using tickets.
Back to Top