#30755 closed New feature (needsinfo)
Have django.views.generic.base.View.as_view() check argument type, via annotations.
Reported by: | David Szotten | Owned by: | nobody |
---|---|---|---|
Component: | Generic views | Version: | dev |
Severity: | Normal | Keywords: | typing |
Cc: | Triage Stage: | Someday/Maybe | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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__
?
Change History (9)
comment:1 by , 5 years ago
Type: | Uncategorized → New feature |
---|
comment:2 by , 5 years ago
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")
comment:3 by , 5 years ago
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.
comment:4 by , 5 years ago
Keywords: | typing added |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
Triage Stage: | Unreviewed → Someday/Maybe |
Version: | 2.2 → master |
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. There's a mailing list discussion for that. I'd rather keep everything together there until we have a path forward.
comment:5 by , 5 years ago
Summary: | Arguments to django.views.generic.base.View.as_view() → Have django.views.generic.base.View.as_view() check argument type, via annotations. |
---|
comment:6 by , 5 years ago
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]
comment:7 by , 5 years ago
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.
comment:8 by , 5 years ago
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)
comment:9 by , 5 years ago
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 )
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 aTypeError
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.