-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Unsafe subtype: datetime/date #20448
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
…em redundant Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
Co-authored-by: m-aciek <[email protected]>
for more information, see https://pre-commit.ci
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
sterliakov
left a comment
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.
Regarding str vs Iterable[str]: are you sure it's actually a good thing to do? I hate pandas decision to prohibit passing a single string as DataFrame columns, it adds unnecessary friction to quickly creating a one-off dataframe (one has to do columns=list('abc')). Is there a reason to reject passing str as Iterable[str] intentionally? Is it a common source of bugs in some codebase you know/work with?
| if self.options and codes.UNSAFE_SUBTYPE in self.options.enabled_error_codes: | ||
| # Block unsafe subtyping relationships when the error code is enabled | ||
| for subclass, superclass in UNSAFE_SUBTYPING_PAIRS: | ||
| if lname == subclass and rname == superclass: | ||
| return False |
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.
| if self.options and codes.UNSAFE_SUBTYPE in self.options.enabled_error_codes: | |
| # Block unsafe subtyping relationships when the error code is enabled | |
| for subclass, superclass in UNSAFE_SUBTYPING_PAIRS: | |
| if lname == subclass and rname == superclass: | |
| return False | |
| if ( | |
| self.options | |
| and codes.UNSAFE_SUBTYPE in self.options.enabled_error_codes | |
| and (lname, rname) in UNSAFE_SUBTYPING_PAIRS | |
| ): | |
| return False |
If I'm not mistaken, you don't actually have to iterate manually for this
| # Each tuple is (subclass_fullname, superclass_fullname). | ||
| # These are cases where a class is a subclass at runtime but treating it | ||
| # as a subtype can cause runtime errors. | ||
| UNSAFE_SUBTYPING_PAIRS: Final = [("datetime.datetime", "datetime.date")] |
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.
This could be a set, but I'm not sure whether it is actually faster than a single-element list
Fixes #9015.
This PR introduces
unsafe-subtypeerror code, which makes mypy check fail for unsafe date and datetime classes inheritance, following @JukkaL's comment in the related issue.Before:
After:
This is Copilot agent's work with my prompts' guidance. I reviewed the produced code. The change includes tests and documentation.
Alternatives considered
Possible addition