-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🏷️ [#1718] Update typehints to use PEP585 generics #1094
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
73 files changed is nice. But it does look like your replace missed the string references in some Optional's and Union's and wrapped the whole replacement in a string. I marked two notable cases.
So I'd search for | None"
en "str |
en | str"
(etc?)
|
||
|
||
@dataclass | ||
class ZaakInformatieObject(ZGWModel): | ||
url: str | ||
informatieobject: Union[str, "InformatieObject"] | ||
zaak: Union[str, Zaak] | ||
informatieobject: "str | InformatieObject" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The union syntax doesn't work with string operands (yet?): python/cpython#90015 (comment).
I'll revert the changes in this file though, because replacing Union
and Optional
seems to cause issues with zgw_consumers.factory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Union/forward reference thing is pretty disappointing. I expected the operator version to be just syntax sugar on Union but apparently not.
header: Optional[ | ||
"JaarOpgaveClientPortTypeSendJaarOpgaveClientInput.Header" | ||
] = field( | ||
header: "JaarOpgaveClientPortTypeSendJaarOpgaveClientInput.Header | None" = field( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
3a9dbb4
to
3ed3432
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1094 +/- ##
===========================================
- Coverage 95.03% 95.02% -0.01%
===========================================
Files 911 911
Lines 32038 31991 -47
===========================================
- Hits 30446 30399 -47
Misses 1592 1592 ☔ View full report in Codecov by Sentry. |
issue: https://taiga.maykinmedia.nl/project/open-inwoner/issue/1718