Update stubs for pytz #3393
Update stubs for pytz #3393
Conversation
third_party/2and3/pytz/__init__.pyi
Outdated
| @@ -29,6 +29,7 @@ class NonExistentTimeError(InvalidTimeError): ... | |||
| utc: _UTCclass | |||
| UTC: _UTCclass | |||
| def timezone(zone: str) -> Union[_UTCclass, _StaticTzInfo, _DstTzInfo]: ... | |||
| def FixedOffset(offset: int) -> Union[_UTCclass, _StaticTzInfo, _DstTzInfo]: ... | |||
There was a problem hiding this comment.
It seems like FixedOffset is an ad-hoc implementation.
If offset is 0, it returns a _UTCclass.
If offset is not 0, it returns a class _FixedOffset. This class is not a subclass of BaseTzInfo, though it is compatible with its interface, except for zone being None. It also implements utcoffset (without a is_dst argument), dst and tzname (returning None). So it doesn't fit with any of the existing classes.
A bit of a mess! I suppose you can add a _FixedTzInfo class which encodes all of that, not sure if it's worth it. Otherwise, you should change the type to Union[_UTCclass, tzinfo].
There was a problem hiding this comment.
Agree with bluetech here. The current return type seems suboptimal, because we generally discourage Union return types, and in fact it doesn't ever return a _StaticTzInfo or a _DstTzInfo.
There was a problem hiding this comment.
👍 Thanks, both. And apologies for the silence - got sucked into other work. I can change the return type to Union[_UTCclass, tzinfo], per bluetech's comment. Will make the change, run the tests locally, and push it up now.
third_party/2and3/pytz/__init__.pyi
Outdated
| @@ -1,4 +1,4 @@ | |||
| from typing import Optional, List, Set, Mapping, Union | |||
| from typing import Optional, List, Set, Mapping, Union, Dict | |||
There was a problem hiding this comment.
You forgot to remove the Dict.
👋 First commit. Coming here from pandas, specifically #28926.
I'm flagging this PR as WIP until I get confirmation on one type I'm unsure of (
_tzinfoskwarg inFixedOffset). I'll open an issue in pytz to confirm that there; and will remove the "WIP" label once that's confirmed.