Added Type Hints #1282
Added Type Hints #1282
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1282 +/- ##
==========================================
- Coverage 89.30% 86.37% -2.94%
==========================================
Files 12 12
Lines 1010 1101 +91
Branches 192 201 +9
==========================================
+ Hits 902 951 +49
- Misses 78 111 +33
- Partials 30 39 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
smithdc1
force-pushed
the
types
branch
3 times, most recently
from
a9a1794
to
40cb9ff
Compare
| self.field = field | ||
| self.attrs = attrs | ||
|
|
||
| def render(self, context): | ||
| def render(self, context: Context) -> str: |
crispy_forms/base.py
Outdated
| from types import TracebackType | ||
| from typing import Collection, Union |
There was a problem hiding this comment.
If you wrap these inside if typing.TYPE_CHECKING: they will only be imported on mypy runs.
There was a problem hiding this comment.
Thanks for the review. 👍
Any other comments on this would be much appreciated. 🙏
smithdc1
force-pushed
the
types
branch
3 times, most recently
from
560e52b
to
d32e765
Compare
crispy_forms/bootstrap.py
Outdated
| template_pack: Union[SimpleLazyObject, str] = TEMPLATE_PACK, | ||
| extra_context: Optional[Dict[str, Any]] = None, |
There was a problem hiding this comment.
I'd create named type aliases for these kind of things, which pop up a lot.
🤔
There was a problem hiding this comment.
Yes. Also I think there will be a lot of "dict contexts". So something like dict[str | int | Node, Any].
I found out this week that dict[str, Any] is not compatible with dict[str | int | Node, Any]. Which I found surprising.
crispy_forms/bootstrap.py
Outdated
| self, | ||
| form: BaseForm, | ||
| context: Context, | ||
| template_pack: Union[str, SimpleLazyObject] = TEMPLATE_PACK, |
There was a problem hiding this comment.
Here's this one again, but the other way round.
(Maybe LazyStr 🤔)
There was a problem hiding this comment.
I need to look at this. 👍
I don't really understand what a simplelazyobject is or why it's used.
| @@ -9,14 +19,19 @@ class KeepContext: | |||
| touch context object themselves, that could introduce side effects. | |||
| """ | |||
|
|
|||
| def __init__(self, context, keys): | |||
| def __init__(self, context: Context, keys: Collection[int | str | Node]) -> None: | |||
There was a problem hiding this comment.
int | str | Node can be replaced with _ContextKeys
Protocal only available in Py3.8+ Added ref to `render_context` fix to django-stubs.
This ensures that imports are correctly guarded behind TYPE_CHECKING
This functionality has been part of Django for some time.
Very speculative, and I'm not even sure we want this, but I've enjoyed the learning journey.
Needs more thought, tidying up, and splitting out into smaller chunks (like the first commit here).
Also likely far too many
# ignoreat the moment. Although some of those are now just waiting for a new release of Django-stubs following a series of contributions there.So yes, still loads to do.
But tests pass (at least locally), so there's that!