Skip to content

Conversation

@bryanforbes
Copy link
Contributor

Currently the method used to import zoneinfo leaves it typed as Any in mypy and Unknown in pyright. This PR updates pendulum.utils._compat to inform type checkers of the correct type information for zoneinfo while leaving the import mechanism the same during runtime. With Timezone having the correct type info from ZoneInfo, a few more type errors surfaced which I took care of either by adding/removing # type: ignore or adding/removing cast() calls. Since no logic has changed at runtime, no unit tests were added.

from backports import zoneinfo
except ImportError:
import zoneinfo # type: ignore[no-redef]
if TYPE_CHECKING:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will having only the TYPE_CHECKING part satisfy mypy? we are using pyupgrade, which understands this type of syntax (unlike try-ImportError) and handles it when upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will having only the TYPE_CHECKING part satisfy mypy? we are using pyupgrade, which understands this type of syntax (unlike try-ImportError) and handles it when upgrading.

If you are asking if the whole if TYPE_CHECKING/else block can be simplified to just the version check condition, the answer is yes. That whole block can be simplified to:

if sys.version_info < (3, 9):
    from backports import zoneinfo
else:
    import zoneinfo

This will work at runtime and during type checking (in both mypy and pyright and I would suspect other type checkers as well). IMO the simplified version is the better solution, but I was trying to make as few changes to the runtime behavior as possible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go with this solution, please. We are going into 3.0 anyway, so it's ok if there are some changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated PR to address this

@bryanforbes bryanforbes force-pushed the bug/fix-timezone-typing branch from e31bfd7 to f183e89 Compare November 24, 2022 20:30
@Secrus Secrus merged commit 46b27c1 into python-pendulum:master Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants