fix #918: remove the need for importlib_metadata in general #929
Conversation
| @@ -17,15 +16,32 @@ | |||
| from ._config import Configuration, ParseFunction | |||
|
|
|||
|
|
|||
| log = _log.log.getChild("entrypoints") | |||
| from importlib.metadata import EntryPoint as EntryPoint | |||
There was a problem hiding this comment.
Should this not be removed? It's outside the version condition.
There was a problem hiding this comment.
Oh, never mind me, I see it's available in Python 3.8 already, and I misread it as the plural.
| @@ -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.
This looks weird. Doesn't this effectively always disable all type checking for this file?
There was a problem hiding this comment.
Type checkers pretend such variables are true
There was a problem hiding this comment.
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.
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.
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.
The unit test is there specifically to check this
If necessary ill add a test with type assertions
(broke python<=3.9)
closes #918