Add type hints to util.py and tracer.py
#1044
Conversation
| self._lock = threading.RLock() | ||
| self._exists = os.path.exists | ||
| self._getenv = os.getenv |
pex/tracer.py
Outdated
| self.msg = msg | ||
| self.verbosity = verbosity | ||
| self.parent = parent | ||
| if parent is not None: | ||
| parent.children.append(self) | ||
| self.children = [] | ||
| parent.children.append(self) # type: ignore[has-type] |
There was a problem hiding this comment.
I couldn't get MyPy happy with this, even with using cast(Trace, parent) and cast("List[Trace]", parent.children). It keeps complaining Cannot determine type of 'children' [has-type].
There was a problem hiding this comment.
Perhaps the idiom note above clears this up. If not, perhaps an isinstance check instead of a not None check. Hopefully the former.
|
|
||
| class DistributionHelper(object): | ||
| @classmethod | ||
| def access_zipped_assets(cls, static_module_name, static_path, dir_location=None): |
There was a problem hiding this comment.
Unused, beyond a test.
There was a problem hiding this comment.
Ah, this old thing. It is used but we don't have API markers yet. This commit's ba58a85 review is lost inside Twitter but was used to allow a Pexed up application that knew about Pexing to extract its assets; i.e: client code called (calls) it. This shouldn't be killed until API / deprecations / etc is set up. Perhaps add a note to this effect since you did the right thing given all the discoverable information.
There was a problem hiding this comment.
Added back. Thanks.
| @@ -86,18 +46,21 @@ def distribution_from_path(cls, path, name=None): | |||
| for dist in find_distributions(path): | |||
| if dist.project_name == name: | |||
| return dist | |||
| return None | |||
There was a problem hiding this comment.
MyPy wanted an explicit return.
| # type: (Optional[Union[str, int, bool]]) -> Optional[Union[str, int, bool]] | ||
| # type: (Optional[_T]) -> Optional[_T] |
pex/tracer.py
Outdated
| parent.children.append(self) | ||
| self.children = [] | ||
| parent.children.append(self) # type: ignore[has-type] | ||
| self.children = cast("List[Trace]", []) |
There was a problem hiding this comment.
This is not idiomatic as I read the mypy docs. You want var = val # type: X for typing variables.
There was a problem hiding this comment.
This fixed the type ignore. Thanks!
pex/tracer.py
Outdated
| self.msg = msg | ||
| self.verbosity = verbosity | ||
| self.parent = parent | ||
| if parent is not None: | ||
| parent.children.append(self) | ||
| self.children = [] | ||
| parent.children.append(self) # type: ignore[has-type] |
There was a problem hiding this comment.
Perhaps the idiom note above clears this up. If not, perhaps an isinstance check instead of a not None check. Hopefully the former.
|
|
||
| class DistributionHelper(object): | ||
| @classmethod | ||
| def access_zipped_assets(cls, static_module_name, static_path, dir_location=None): |
There was a problem hiding this comment.
Ah, this old thing. It is used but we don't have API markers yet. This commit's ba58a85 review is lost inside Twitter but was used to allow a Pexed up application that knew about Pexing to extract its assets; i.e: client code called (calls) it. This shouldn't be killed until API / deprecations / etc is set up. Perhaps add a note to this effect since you did the right thing given all the discoverable information.
tests/test_integration.py
Outdated
| @@ -994,20 +994,26 @@ def test_pex_multi_resolve_2(): | |||
| ), "{} was not found in wheel".format(dist_substr) | |||
|
|
|||
|
|
|||
| if TYPE_CHECKING: | |||
| TEST_RESOLVE_FN = Callable[[str, str, str, str, Optional[str]], None] | |||
There was a problem hiding this comment.
All caps type names are jarring. I'm guessing you're trying to distinguish an alias from a "real" type via hungarian - but, by definition this is already all local to the file so I'm not seeing the point.
There was a problem hiding this comment.
My thinking was it was a constant, so following the convention. But I like PascalCase more here.
Also clean up some prior type hints to use
cast, type vars, and type aliases.