Clarify Base<FOO>View usage in docstrings.

[Jesús Leganés-Combarro]

At ​https://github.com/django/django/blob/a57596e443ecb67140e1a9fc0f0e87446b2d0174/django/views/generic/list.py#L175, BaseListView.get() method is calling to self.render_to_response(context) method. Fact is, I have searched in all the BaseListView ancestor classes (MultipleObjectMixin, ContextMixin and View) and was not able to find any reference to the render_to_response() method, both in the documentation or the code itself. The only place I have been able to find it implemented is in the TemplateResponseMixin class, that's templates oriented, and it's only related to BaseListView because both classes are ancestors of ListView.

In its current state, using BaseListView class without overriding the BaseListView.get() method (that already implements a proper handler for HTTP GET method, so overriding would not be needed) would crash because render_to_response() method can't be found. One solution would be to add render_to_response() as an abstract method, and add it on the documentation forcing to be implemented in a child class, but render_to_response() feels to be too much related to templates, and was not able to find any other rendering related function in View or other ancestor class of BaseListView... Another maybe better solution would be to move BaseListView.get() method to ListView.get(), left ing BaseListView empty the same way that ListView currently is, or just with a get_context_data() method to include the checking if list is empty like a validation, but that last ones would be backward incompatibles... Is there any other better solution for this?

[Clifford Gama]

From reading the code, it looks like the BaseListView like the other Base<FOO>Views are not meant to be subclassed.
I tried creating a view inheriting the BaseDetailView , and I got the same error:

'MyBaseDetailView' object has no attribute 'render_to_response'

This line is missing from the docstring in BaseListView and the BaseDetailView.

"Using this base class requires subclassing to provide a response mixin."

And I think that needs to be added. But you may have to check if other Base<FOO>View also need that documentation.

[Jesús Leganés-Combarro]

Adding the "Using this base class requires subclassing to provide a response mixin" comment in the Docstring would be really nice, but in addition to that, I would make it more explicit by adding an implementation of render_to_response() that raise a NotImplementedError exception, that at least would provide a more specific message than a generic "object has no attribute" one.

Besides that, although render_to_response is focused on templates, is it also a good name for non-template-based Views? For example in my use case I'm generating a dict object with a GeoJSON FeaturesCollection object and later setting it as data of a JsonResponse object, there are no templates involved at all.

[YashRaj1506]

Replying to Jesús Leganés-Combarro:

Adding the "Using this base class requires subclassing to provide a response mixin" comment in the Docstring would be really nice, but in addition to that, I would make it more explicit by adding an implementation of render_to_response() that raise a NotImplementedError exception, that at least would provide a more specific message than a generic "object has no attribute" one.

Besides that, although render_to_response is focused on templates, is it also a good name for non-template-based Views? For example in my use case I'm generating a dict object with a GeoJSON FeaturesCollection object and later setting it as data of a JsonResponse object, there are no templates involved at all.

Hey i am trying to solve this issue, i wanted to project this solution, One way is that the ListView is inheriting from two classes called MultipleObjectTemplateResponseMixin and BaseListView and MultipleObjectTemplateResponseMixin inherits from TemplateResponseMixin, which contains the function render_to_response so eventually when BaseListView.get() is used in ListView the render_to_reponse method is available there from MultipleObjectTemplateResponseMixin and hence works with no problem, but if the same thing happens while using BaseListView we will obviously get error because TemplateResponseMixin is not inheritied. So one way is We can add a docstring that this class(BaseListView) is meant to be subclassed by ListView, so use ListView and let BaseListView be just a base class meant to be inherited (we will raise a error too) .Would like to know your opinion, accordingly i will make the pr.

[Clifford Gama]

So one way is We can add a docstring that this class(BaseListView) is meant to be subclassed by ListView, so use ListView and let BaseListView be just a base class meant to be inherited (we will raise a error too) . Would like to know your opinion, accordingly i will make the pr.

I don't think that's quite accurate. While it is true that ListView subclasses BaseListView, it does not follow that BaseListView must always be subclassed by ListView.

I think better is:

Using this base class requires subclassing to provide a response mixin.

which I copied from BaseUpdateView.

Btw, you may want to look at other BaseFOOViews to see how they handle that, and also if other abstract base views are also missing that in their docstring.

[YashRaj1506]

Replying to Clifford Gama:

So one way is We can add a docstring that this class(BaseListView) is meant to be subclassed by ListView, so use ListView and let BaseListView be just a base class meant to be inherited (we will raise a error too) . Would like to know your opinion, accordingly i will make the pr.

I don't think that's quite accurate. While it is true that ListView subclasses BaseListView, it does not follow that BaseListView must always be subclassed by ListView.

I think better is:

Using this base class requires subclassing to provide a response mixin.

which I copied from BaseUpdateView.

Btw, you may want to look at other BaseFOOViews to see how they handle that, and also if other abstract base views are also missing that in their docstring.

Okay sure, i will look through other BaseFOOViews and will return to you with a final proposal. Thanks on the insights.

[YashRaj1506]

Replying to Clifford Gama:

So one way is We can add a docstring that this class(BaseListView) is meant to be subclassed by ListView, so use ListView and let BaseListView be just a base class meant to be inherited (we will raise a error too) . Would like to know your opinion, accordingly i will make the pr.

I don't think that's quite accurate. While it is true that ListView subclasses BaseListView, it does not follow that BaseListView must always be subclassed by ListView.

I think better is:

Using this base class requires subclassing to provide a response mixin.

which I copied from BaseUpdateView.

Btw, you may want to look at other BaseFOOViews to see how they handle that, and also if other abstract base views are also missing that in their docstring.

I checked the other Base<FOO>View and yes as written in BaseUpdateView inside docstring

Using this base class requires subclassing to provide a response mixin.

this makes totally sense and while i checked BaseDetailView and BaseListView adding this very line would make things clear (As BaseDetailView is also using render_to_response from TemplateResponseMixin`) so my final proposal would be to add this

Using this base class requires subclassing to provide a response mixin.

inside the docstring of BaseDetailView and BaseListView.

I have made the above mentioned changes, is there anything else you would like to suggest otherwise i find it fine now and will make the Pr soon.

[Clifford Gama]

I think that's okay for now. You'll get more suggestions as necessary from reviewers in the PR

[Jesús Leganés-Combarro]

In addition of the DocString, what about adding an implementation of render_to_response() that raise NotImplementedError() to make the error explicit in runtime?

[Clifford Gama]

Replying to Jesús Leganés-Combarro:

In addition of the DocString, what about adding an implementation of render_to_response() that raise NotImplementedError() to make the error explicit in runtime?

Sounds like a good idea, but I wasn't sure about it since no NotImplementedError() are raised in any of the other BaseFOOViews and most other Mixins. I think we will have to ask about this in the Django Internals Forum.

[Jesús Leganés-Combarro]

Replying to Clifford Gama:

Replying to Jesús Leganés-Combarro:

In addition of the DocString, what about adding an implementation of render_to_response() that raise NotImplementedError() to make the error explicit in runtime?

Sounds like a good idea, but I wasn't sure about it since no NotImplementedError() are raised in any of the other BaseFOOViews and most other Mixins. I think we will have to ask about this in the Django Internals Forum.

I would personally add it in all the BaseFOOViews, it's a bit nasty to call a method that doesn't exist just because it's expected to be implemented in a subclass.

OTOH, doing so would break compatibility in the cases where it's not implemented in a subclass, but in another sibling class in the MRO, so it would break in case the sibling class implementing it is defined after the BaseFOOViews with the implementation raising the exception, but IMHO, that's already broken code that's sucesfully running just by luck they are defined in the correct MRO order.

[Carlton Gibson]

Forum discussion for this issue: ​https://forum.djangoproject.com/t/notimplementederror-on-basefooview/33978/1

[YashRaj1506]

PR has been made : ​https://github.com/django/django/pull/18540