Skip to content
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

Typing annotations fixed in pkg_resources/_vendor/packaging/version.py #4044

Closed
wants to merge 1 commit into from

Conversation

jolaf
Copy link

@jolaf jolaf commented Sep 10, 2023

Summary of changes

I've run my app under typeguard and it found couple of typing annotation issues that I fix here.

Closes

Pull Request Checklist

@jolaf
Copy link
Author

jolaf commented Sep 10, 2023

See also pypa/packaging#723

@abravalheri
Copy link
Contributor

Hi @jolaf, thank you very much for the contribution.

Could you please identify where the type annotation is giving you problems? Is it something inside the setuptools source.

Setuptools vendors the packaging project (and everything inside the _vendor folder), so we don't do arbitrary patches because otherwise, it becomes very difficult to sync.

If the packaging maintainers find the patch relevant and add it to a future version, we can update the vendored version of packaging in the future.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Could you please identify where the type annotation is giving you problems? Is it something inside the setuptools source.

I run my app using pkg_resources under typeguard. The app crashes because the actual values passed into functions has types different from the ones specified in annotations. So I'm here correcting the annotations to reflect that. If you check the code you would see that those arguments are checked for None, that's because None is sometimes passed as those arguments, and annotations do not reflect that. That's what I've fixed here.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

For example, test.py:

from typeguard import install_import_hook
with install_import_hook(('pkg_resources',)):
    import pkg_resources

results in:

$ python3 test.py 
Traceback (most recent call last):
  File "/media/sf_d/WWPass/git/test.py", line 3, in <module>
    import pkg_resources
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "/home/jolaf/.local/lib/python3.10/site-packages/typeguard/_importhook.py", line 98, in exec_module
    super().exec_module(module)
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 3328, in <module>
    def _initialize_master_working_set():
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 3302, in _call_aside
    f(*args, **kwargs)
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 3340, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 622, in _build_master
    ws = cls()
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 615, in __init__
    self.add_entry(entry)
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 671, in add_entry
    for dist in find_distributions(entry, True):
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 2134, in find_on_path
    for dist in factory(fullpath):
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 2199, in distributions_from_metadata
    yield Distribution.from_location(
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 2665, in from_location
    return cls(
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 2646, in __init__
    self._version = safe_version(version)
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/__init__.py", line 1398, in safe_version
    return str(packaging.version.Version(version))
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/_vendor/packaging/version.py", line 209, in __init__
    local=_parse_local_version(match.group("local")),
  File "/home/jolaf/.local/lib/python3.10/site-packages/pkg_resources/_vendor/packaging/version.py", line 492, in _parse_local_version
    def _parse_local_version(local: str) -> Optional[LocalType]:
  File "/home/jolaf/.local/lib/python3.10/site-packages/typeguard/_functions.py", line 138, in check_argument_types
    check_type_internal(value, annotation, memo)
  File "/home/jolaf/.local/lib/python3.10/site-packages/typeguard/_checkers.py", line 766, in check_type_internal
    raise TypeCheckError(f"is not an instance of {qualified_name(origin_type)}")
typeguard.TypeCheckError: argument "local" (None) is not an instance of str

@abravalheri
Copy link
Contributor

Thank you very much @jolaf for the information.

In your traceback, it seems that pkg_resources itself is not offending any type annotation, so there is not much we can do in the pkg_resources codebase itself.

Regarding how packaging internally handles the regex, I think the best we can do in the pypa/setuptools repository is wait to see if packaging releases a new version and then we can update the _vendor folder. Because of that I will close this PR for now, but I will keep an eye for packaging releases. Please feel free to also ping me again in the future if a new packaging release is made.

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.

None yet

2 participants