Better reflect non-datetime uses with tzinfo

[pganssle]

The actual tzinfo interface allows for passing a time as well as None to the three main abstract methods, and it expects that when you pass it a datetime you will always get a valid answer (whereas for time and None it depends on whether or not the zone represents a fixed offset).

I am not sure if this is the right way to stack overload and abstractmethod decorators.

[pganssle]

Hmm...

+ sphinx/builders/gettext.py: note: In member "dst" of class "LocalTimeZone":
+ sphinx/builders/gettext.py:192:5: error: Signature of "dst" incompatible with supertype "tzinfo"  [override]
+ sphinx/builders/gettext.py:192:5: note:      Superclass:
+ sphinx/builders/gettext.py:192:5: note:          @overload
+ sphinx/builders/gettext.py:192:5: note:          def dst(self, datetime, /) -> timedelta
+ sphinx/builders/gettext.py:192:5: note:          @overload
+ sphinx/builders/gettext.py:192:5: note:          def dst(self, Optional[time], /) -> Optional[timedelta]
+ sphinx/builders/gettext.py:192:5: note:      Subclass:
+ sphinx/builders/gettext.py:192:5: note:          def dst(self, dt: Optional[datetime]) -> timedelta

Not sure I fully understand the situation here. It seems like dst(self, dt: Optional[datetime]) -> timedelta is a valid choice for an implementation, except that it says it doesn't support datetime.time. Is that the reason this is failing, or is the issue that it doesn't understand that the cases in the superclass are:

1. datetime -> timedelta
2. None -> timedelta
3. None -> None

And the cases in the subclass are:

1. datetime -> timedelta
3. None -> timedelta

Seems compatible to me.

If it's the time thing, that feels like it could be tricky, since right now valid uses of tzinfo are forbidden by the typeshed stubs (e.g. how do you even use the tzinfo parameter to datetime.time?), but adding that to the signature will break any subclasses that haven't enumerated it.

[AlexWaygood]

If a superclass says that it will accept a datetime.time for a parameter in some circumstances, but a subclass says that it doesn't accept a datetime.time under any circumstances for the same parameter, then that violates the Liskov Substitution Principle. You can see the lack of type safety in this simplified example here:

class Foo:
    def method(self, obj: str | None) -> None:
        if obj is not None:
            print(obj.upper())

class Bar(Foo):
    def method(self, obj: str) -> None:
        print(obj.upper())

foo_instance: Foo = Bar()
foo_instance.method(None)  # Oh no

Note that the mypy_primer results are non-blocking (we can probably merge anyway if we judge that other people will just need to change their type annotations to reflect the new typeshed stubs). The mypy failures in CI are blocking, however, so should probably be looked at first. You'll either need to change the signatures of the various tzinfo subclasses typeshed has stubs for, or (if that's not a suitable option for a certain subclass) add a # type: ignore[override] to the problematic method definitions.

[pganssle]

If a superclass says that it will accept a datetime.time for a parameter in some circumstances, but a subclass says that it doesn't accept a datetime.time under any circumstances for the same parameter, then that violates the Liskov Substitution Principle.

OK, so in this case I think that this will just be a breaking change, since the actual tzinfo interface does allow you to pass it a datetime.time. Should we just ignore the breakages in third party types, then, and focus on the typeshed breakage, which appears to be this:

stdlib/zoneinfo/__init__.pyi:31: error: Signature of "dst" incompatible with supertype "tzinfo"  [override]
stdlib/zoneinfo/__init__.pyi:31: note:      Superclass:
stdlib/zoneinfo/__init__.pyi:31: note:          def dst(self, Union[datetime, time, None], /) -> Optional[timedelta]
stdlib/zoneinfo/__init__.pyi:31: note:      Subclass:
stdlib/zoneinfo/__init__.pyi:31: note:          @overload
stdlib/zoneinfo/__init__.pyi:31: note:          def dst(self, datetime, /) -> timedelta
stdlib/zoneinfo/__init__.pyi:31: note:          @overload
stdlib/zoneinfo/__init__.pyi:31: note:          def dst(self, Optional[time], /) -> Optional[timedelta]

Not sure why that's not considered compatible, since the subclass does accept datetime, None and time, and returns either timedelta or None, its just that the subclass is being more specific about some of the situations where you would never get a None.

[hauntsaninja]

In general, typeshed is okay with forcing people who subclass types to update when we improve the types, so the two new primer hits from that in sphinx and mongodb aren't the worst.

That said, there's definitely a mypy bug here. If we fix that, it'll be the happiest way forward, but I'm fine with type ignore and moving on. I filed python/mypy#15121 for this

[JelleZijlstra]

Suggested change

	def tzname(self, __dt: time | None) -> str: ...
	def tzname(self, __dt: time | None) -> str | None: ...

Currently the two overloads return the same type

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

streamlit (https://github.com/streamlit/streamlit)
+ lib/tests/streamlit/runtime/caching/hashing_test.py:32: note: ... from here:
+ scripts/pypi_nightly_create_tag.py:24: note: ... from here:
+ lib/tests/streamlit/runtime/caching/hashing_test.py:32: note: ... from here:
+ scripts/pypi_nightly_create_tag.py:24: note: In module imported here:

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ bson/tz_util.py:42: error: Signature of "utcoffset" incompatible with supertype "tzinfo"  [override]
+ bson/tz_util.py:42: note:      Superclass:
+ bson/tz_util.py:42: note:          @overload
+ bson/tz_util.py:42: note:          def utcoffset(self, datetime, /) -> timedelta
+ bson/tz_util.py:42: note:          @overload
+ bson/tz_util.py:42: note:          def utcoffset(self, Optional[time], /) -> Optional[timedelta]
+ bson/tz_util.py:42: note:      Subclass:
+ bson/tz_util.py:42: note:          def utcoffset(self, dt: Optional[datetime]) -> timedelta
+ bson/tz_util.py:45: error: Signature of "tzname" incompatible with supertype "tzinfo"  [override]
+ bson/tz_util.py:45: note:      Superclass:
+ bson/tz_util.py:45: note:          @overload
+ bson/tz_util.py:45: note:          def tzname(self, datetime, /) -> str
+ bson/tz_util.py:45: note:          @overload
+ bson/tz_util.py:45: note:          def tzname(self, Optional[time], /) -> Optional[str]
+ bson/tz_util.py:45: note:      Subclass:
+ bson/tz_util.py:45: note:          def tzname(self, dt: Optional[datetime]) -> str
+ bson/tz_util.py:48: error: Argument 1 of "dst" is incompatible with supertype "tzinfo"; supertype defines the argument type as "Union[datetime, time, None]"  [override]
+ bson/tz_util.py:48: note: This violates the Liskov substitution principle
+ bson/tz_util.py:48: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+ bson/json_util.py:925: error: Unused "type: ignore" comment  [unused-ignore]

[JelleZijlstra]

Thanks for contributing! I'm closing this PR for now, because it still
fails some tests and has unresolved review feedback
after three months of inactivity. If you are still interested, please feel free to open
a new PR (or ping us to reopen this one).