-
-
Notifications
You must be signed in to change notification settings - Fork 216
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
fix #918: remove the need for importlib_metadata in general #929
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,12 @@ | |
".py": """\ | ||
# file generated by setuptools_scm | ||
# don't change, don't track in version control | ||
from __future__ import annotations | ||
TYPE_CHECKING = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks weird. Doesn't this effectively always disable all type checking for this file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type checkers pretend such variables are true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, this is not normal way to do this. Normally you import typing and check typing.TYPE_CHECKING. It is normally never set by user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a trick specifically for the type checkers they use the variable name as indication, that way one does not have to do a runtime import as importing typing can be expensive , its deemed that the version file ought not to impose that cost so this really is just a dirty trick to avoid runtime imports in type check able files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure this really works? Have you tested type checkers fail here if you provide wrong input? https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING says this is specifically a constant in typing module. Mypy documentation agrees https://mypy.readthedocs.io/en/stable/runtime_troubles.html#import-cycles There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unit test is there specifically to check this If necessary ill add a test with type assertions |
||
if TYPE_CHECKING: | ||
from typing import Tuple | ||
|
||
__version__ = version = {version!r} # type: str | ||
__version_tuple__ = version_tuple = {version_tuple!r} # type: tuple[int | str, ...] | ||
__version_tuple__ = version_tuple = {version_tuple!r} # type: Tuple[int | str, ...] | ||
""", | ||
".txt": "{version}", | ||
} | ||
|
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.
Should this not be removed? It's outside the version condition.
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.
Oh, never mind me, I see it's available in Python 3.8 already, and I misread it as the plural.