-
-
Notifications
You must be signed in to change notification settings - Fork 454
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
More precise annotations of utils.timezone functions that return instances of tzinfo subclasses. #209
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.
I also ran into this issue, will be good to fix. Left some comments.
It looks good to me now, though I would add a comment like this above
|
OK @bluetech It's included as suggested. Also, what is breaking the build? I see the output, but what is the difference with the master that causes problems? |
Yes, I will be able to fix it in the evening (Moscow time). Sorry for the inconvenience. |
Something broke during this time period: My wild guess is that
At first sight - it does not look like something has changed at all. Local build also fails with the very same reason. Then, I have tried to update --- Subproject commit f452d4232e1c14269bf2f1bd234d6e4ef462dc55
+++ Subproject commit aa28213eb51ba26b349c2e51f2defcc7cc87780e Still no luck, the error remains. I am out of ideas. And sadly don't have any more time for now. |
@sobolevn FYI, CI failure happened because of unpinned mypy version. 0.740 broke tests typechecking. |
That's why I use |
Current annotations for
get_fixed_timezone
,get_default_timezone
andget_current_timezone
follow the docstrings of the functions that broadly name returned objects as tzinfo instance. It is true, but imprecise, as the returned objects are instances of subclasses of tzinfo. Mypy complains when any of the methods of that subclasses are called on the returned instance eg.error: "tzinfo" has no attribute "localize"
Proposed changes should fix it.