Opened 5 years ago

Closed 3 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: master
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 5 years ago.
relocate_view_class-2.diff (13.8 KB) - added by Preston Holmes 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by Preston Holmes

Attachment: relocate_view_class.diff added

comment:1 Changed 5 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
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.

Changed 5 years ago by Preston Holmes

Attachment: relocate_view_class-2.diff added

comment:2 Changed 5 years ago by Preston Holmes

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 Changed 5 years ago by Bas Peschier

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.

comment:4 in reply to:  3 Changed 5 years ago by Preston Holmes

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 Changed 5 years ago by Preston Holmes

Owner: changed from nobody to Preston Holmes
Status: newassigned

comment:6 Changed 4 years ago by Aymeric Augustin

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 Changed 3 years ago by Tim Graham

Patch needs improvement: set

No longer applies cleanly.

comment:8 Changed 3 years ago by Preston Holmes

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