Code

Opened 3 years ago

Closed 12 months ago

#16963 closed Cleanup/optimization (wontfix)

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

Reported by: ptone Owned by: ptone
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 ptone 3 years ago.
relocate_view_class-2.diff (13.8 KB) - added by ptone 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by ptone

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 3 years ago by ptone

comment:2 Changed 3 years ago by ptone

  • 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 follow-up: Changed 3 years ago by 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.

comment:4 in reply to: ↑ 3 Changed 3 years ago by ptone

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 3 years ago by ptone

  • Owner changed from nobody to ptone
  • Status changed from new to assigned

comment:6 Changed 2 years ago by aaugustin

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 12 months ago by timo

  • Patch needs improvement set

No longer applies cleanly.

comment:8 Changed 12 months ago by ptone

  • Resolution set to wontfix
  • Status changed from assigned to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.