[Django] #30755: Arguments to django.views.generic.base.View.as_view()
Django
-----------------------------------------+------------------------
Reporter: David Szotten | Owner: nobody
Type: Uncategorized | Status: new
Component: Generic views | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
`django.views.generic.base.View.as_view()` raises a `TypeError` if passed
a kwarg that's not already an attribute to the class
https://github.com/django/django/blame/master/django/views/generic/base.py#L57
From the git blame i can't tell what the reason for this is. Is it only to
catch potential mistakes/typos?
If so, would we be open to also accepting attributes in
`cls.__annotations__`?
--
Ticket URL: <https://code.djangoproject.com/ticket/30755>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+--------------------------------------
Reporter: David Szotten | Owner: nobody
Component: Generic views | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* type: Uncategorized => New feature
Comment:
I think we should keep the `TypeError` behaviour as it's an effective way
of catching typos and follows the configurable pattern.
If we were adding support fallback for `__annotations__` I believe a
`TypeError` should be raised if no value is provided.
Not sure it's worth doing though as `foo: annotation = None` is a good way
to provide a default and an annotation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:1>
Django
-------------------------------+--------------------------------------
Component: Generic views | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Szotten):
the downside of `foo: annotation = None` is that it doesn't type check:
`Incompatible types in assignment (expression has type "None", variable
has type "str")`
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:2>
Django
-------------------------------+--------------------------------------
Component: Generic views | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Not sure I understand the issue here, can't you use `Optional[str]` in
this case? Django doesn't support typing yet so I'm not sure of the origin
of this type check failure.
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:3>
Django
-------------------------------+-----------------------------------------
Reporter: David Szotten | Owner: nobody
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* keywords: => typing
* version: 2.2 => master
* resolution: => needsinfo
* stage: Unreviewed => Someday/Maybe
Comment:
> Django doesn't support typing yet...
Yeah, I think this is the one.
Type hints seem to have come on a long way in recent versions. The
benefits seem to be summing up. So I'm sure they'll come to Django.
But at current state of play, I'd be **very surprised** if `as_view()`
raised a `TypeError` here for that.
I'll mark this as Someday/Maybe but close it as `needsinfo`: all things
typing are dependent on a DEP being produced and agreed on.
[https://groups.google.com/d/topic/django-
developers/trTEbURFhEY/discussion There's a mailing list discussion for
that]. I'd rather keep everything together there until we have a path
forward.
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:4>
Django
annotations.
-------------------------------+-----------------------------------------
Reporter: David Szotten | Owner: nobody
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:5>
Django
annotations.
-------------------------------+-----------------------------------------
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Szotten):
I'm happy to leave this to the linked mailing list discussion/dep
In the mean time, would you accept a patch pulling out the `initkwargs`
validation from `as_view` into a helper method, to make it easier for me
to override this behaviour in my projects?
Are you able to confirm that the only use-case for the validation is to
catch likely mistakes/typos? Not to diminish it, i think it's very
valuable, but i'm trying to make sure I'm not breaking something if i
override this behaviour. (In a type-annotated codebase I think a type
annotation is an equally valid "statement of intent")
For some more context,
the reason I'd rather not make it `Optional` is that it isn't really and
I'd like the type checker to know that this value is always say a `str`
and not make me check for `None`.
{{{
class MyView(View):
key: str
def get(self, request):
# at this point self.key is definitely a str, not an Optional[str]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:6>
Django
annotations.
-------------------------------+-----------------------------------------
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
> In the mean time, would you accept a patch pulling out the initkwargs
validation from as_view into a helper method, to make it easier for me to
override this behaviour in my projects?
TBH, I'd prefer that you override, do your extra validation and then call
`super()`, rather than adding an additional hook, and all that goes with
it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:7>
Django
annotations.
-------------------------------+-----------------------------------------
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Szotten):
sorry if i was unclear, but i think part of the original intent got lost.
the point is that we also need to _remove_ (well, relax) some of the
existing validation, because it checks that all initkwargs are attributes
on the class, and `key` in my example above isn't ("pure" annotations
don't become attributes on the class)
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:8>
Django
annotations.
-------------------------------+-----------------------------------------
Component: Generic views | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: typing | Triage Stage: Someday/Maybe
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Ah, yes. I see. Sorry for not following you. ...
More or less, for me, I'd rather see a actual solution that works for
everyone, than a hook to override. (Ultimately bitty APIs don't help
anyone...) So I'd rather see effort going into pushing forward the typing
efforts.
(In the meantime I'd point you to a mixin or such for your own needs — I
know that's not ideal but ^^^^)
--
Ticket URL: <https://code.djangoproject.com/ticket/30755#comment:9>