fix a bunch of typing errors #4024

danieleades · 2021-05-02T19:59:33Z

this pull request takes the total number of mypy errors from 656 to 617

it's a start...

abn · 2021-05-06T00:09:57Z

I do not think most of these are required. The only one that I can see as being the right change is at 943fb35#diff-89c37d9db9d559ea6d241ff3fcbee2053643e688e99a177327fd0ad15580ba26R239.

danieleades · 2021-05-06T05:41:01Z

I do not think most of these are required. The only one that I can see as being the right change is at 943fb35#diff-89c37d9db9d559ea6d241ff3fcbee2053643e688e99a177327fd0ad15580ba26R239.

These are all required in the sense that the original code is an error with respect to the current mypy configuration.

The return errors could be suppressed with a config change.

There's a number of cases where I've swapped variant types for invariant types. This is required for correctness. See https://mypy.readthedocs.io/en/latest/generics.html#variance-of-generic-types

danieleades · 2021-05-06T06:05:01Z

poetry/installation/installer.py

    def _populate_local_repo(
-        self, local_repo: Repository, ops: List[Operation]
+        self, local_repo: Repository, ops: Sequence[Operation]


@abn
here's an example of the use of a 'covariant' type (required for correctness)

the _populate_local_repo method accepts ops: List[Operation], however Operation is a base class. In practice we expect that this method should accept a list of some derived class (such as Uninstall, for example).

the problem is that List[Subclass] is not equivalent to List[Baseclass] (see these docs - http://mypy.readthedocs.io/en/latest/generics.html#variance-of-generic-types)

however Sequence[Subclass] is equivalent to Sequence[Baseclass]. Similarly, Iterable[Subclass] is equivalent to Iterable[Baseclass].

danieleades · 2021-05-06T06:08:44Z

poetry/utils/extras.py

-) -> Iterator[str]:
+) -> Iterable[str]:


@abn
this too is a correctness bug. the returned value of this function is not an iterator, but it is a type than can be converted into an iterator. That's the distinction between Iterator and Iterable

danieleades · 2021-05-06T06:51:33Z

poetry/utils/env.py

            return Path(self.paths["usersite"])
+        return None


@abn
explicitly returning None is required to conform with PEP8 and to satisfy mypy

from PEP8-

Be consistent in return statements. Either all return statements in a function should return an expression, or none of them should. If any return statement returns an expression, any return statements where no value is returned should explicitly state this as return None, and an explicit return statement should be present at the end of the function (if reachable)

it's not without controversy, see this discussion - python/mypy#7511

it depends what the goal is. The goal of this pull request is not strictly correctness, it is to reduce the amount of noise that mypy makes, in the hope that this is a small step in the direction of actually being able to effectively use Mypy to lint this codebase (and also correctness!).

In a perfect world, the number of type errors would be 0, and then mypy could be added as a pre-commit hook or CI job to ensure type errors are not reintroduced. Baby steps.

danieleades · 2021-05-09T12:35:11Z

poetry/console/commands/env_command.py

-if TYPE_CHECKING:
-    from poetry.utils.env import Env


this pattern is used in the case that an unconditional import would cause a circular import. This isn't required here.

danieleades · 2021-05-09T12:37:42Z

poetry/mixology/incompatibility.py

-    def _single_term_where(self, callable: callable) -> Optional[Term]:
+    def _single_term_where(self, callable: Callable[[Term], bool]) -> Optional[Term]:


the built-in callable can't be used for type annotations

danieleades · 2021-05-09T12:39:06Z

poetry/packages/project_package.py

-    ) -> "ProjectPackage":
+    ) -> None:


this method doesn't return...

danieleades · 2021-05-09T12:40:39Z

poetry/utils/env.py

-    def __eq__(self, other: "Env") -> bool:
+    def __eq__(self, other: object) -> bool:


this is required to satisfy Liskov subsitution principle- namely that the derived class's __eq__ should have the same, or a less restrictive function signature for any overridden baseclass methods.

danieleades · 2021-05-09T12:41:26Z

poetry/utils/env.py

-    def _run(self, cmd: List[str], **kwargs: Any) -> int:
+    def _run(self, cmd: List[str], **kwargs: Any) -> Optional[int]:


this change, while correct, actually increases the total number of errors, since the None case is not handled in the calling code.

danieleades · 2021-05-09T12:42:00Z

poetry/utils/shell.py

@@ -61,7 +62,7 @@ def get(cls) -> "Shell":

        return cls._shell

-    def activate(self, env: VirtualEnv) -> None:
+    def activate(self, env: VirtualEnv) -> Optional[int]:
        if WINDOWS:
            return env.execute(self.path)


this method returns Optional[int]

danieleades · 2021-05-13T19:02:26Z

Ping @abn

sonarcloud · 2021-06-03T08:11:59Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

danieleades · 2021-06-03T08:12:25Z

@abn do you mind approving the workflows?

danieleades · 2021-09-15T15:50:30Z

rebased. @abn, it's that time again...

skovhus

This looks very solid and I cannot see any controversial changes here. I hope this gets merged soon.

danieleades · 2021-09-26T05:55:53Z

This looks very solid and I cannot see any controversial changes here. I hope this gets merged soon.

i'm not holding my breath...

danieleades · 2021-09-26T07:39:48Z

@abn there's over 700 typing errors now in this repo, even after these changes. Is there any appetite for reducing/resolving these?

neersighted

Thanks for this work! I'm very interesting in getting the typing issues reduced and getting mypy into the Code Quality checks. Feel free to reach out offline (e.g. Discord) to coordinate on this.

github-actions · 2024-02-29T21:08:49Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

danieleades commented May 6, 2021

View reviewed changes

danieleades force-pushed the refactor/typing branch from 943fb35 to 76f75f6 Compare May 9, 2021 09:05

danieleades commented May 9, 2021

View reviewed changes

poetry/packages/project_package.py

) -> "ProjectPackage":

) -> None:

Copy link

Contributor Author

danieleades May 9, 2021

Choose a reason for hiding this comment

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

this method doesn't return...

danieleades commented May 9, 2021

View reviewed changes

danieleades force-pushed the refactor/typing branch 2 times, most recently from 2e1d4c1 to d22069f Compare June 3, 2021 08:10

danieleades force-pushed the refactor/typing branch from d22069f to 9dd8518 Compare September 15, 2021 15:47

danieleades mentioned this pull request Sep 15, 2021

Set up mypy pre-commit hook for incremental adoption #4510

Merged

skovhus reviewed Sep 25, 2021

View reviewed changes

danieleades force-pushed the refactor/typing branch from 7e58c9f to acdfd79 Compare September 26, 2021 05:57

danieleades mentioned this pull request Sep 26, 2021

mypy: fix missing return statement/value errors #3801

Closed

danieleades added 5 commits October 8, 2021 10:34

fix a bunch of typing errors

5ebf24c

fix more type errors

db3a6fc

fix more type errors

132905c

fix run failure

e6d4d96

fix more type errors

4a20fe4

fix more typings

390c47e

danieleades force-pushed the refactor/typing branch from acdfd79 to 390c47e Compare October 8, 2021 09:34

neersighted approved these changes Nov 13, 2021

View reviewed changes

neersighted merged commit a49372a into python-poetry:master Nov 13, 2021

1nF0rmed pushed a commit to 1nF0rmed/poetry that referenced this pull request Nov 15, 2021

fix: misc type errors (python-poetry#4024)

b0e5a49

edvardm pushed a commit to edvardm/poetry that referenced this pull request Nov 24, 2021

fix: misc type errors (python-poetry#4024)

d7fdeeb

github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix a bunch of typing errors #4024

fix a bunch of typing errors #4024

danieleades commented May 2, 2021

abn commented May 6, 2021

danieleades commented May 6, 2021

danieleades May 6, 2021

danieleades May 6, 2021

danieleades May 6, 2021

danieleades May 9, 2021

danieleades May 9, 2021

danieleades May 9, 2021

danieleades May 9, 2021

danieleades May 9, 2021

danieleades May 9, 2021 •

edited

Loading

danieleades commented May 13, 2021

sonarcloud bot commented Jun 3, 2021

danieleades commented Jun 3, 2021

danieleades commented Sep 15, 2021

skovhus left a comment

danieleades commented Sep 26, 2021

danieleades commented Sep 26, 2021

neersighted left a comment

github-actions bot commented Feb 29, 2024

		def _single_term_where(self, callable: callable) -> Optional[Term]:
		def _single_term_where(self, callable: Callable[[Term], bool]) -> Optional[Term]:

		def __eq__(self, other: "Env") -> bool:
		def __eq__(self, other: object) -> bool:

		def _run(self, cmd: List[str], **kwargs: Any) -> int:
		def _run(self, cmd: List[str], **kwargs: Any) -> Optional[int]:

fix a bunch of typing errors #4024

fix a bunch of typing errors #4024

Conversation

danieleades commented May 2, 2021

abn commented May 6, 2021

danieleades commented May 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieleades May 9, 2021 • edited Loading

Choose a reason for hiding this comment

danieleades commented May 13, 2021

sonarcloud bot commented Jun 3, 2021

danieleades commented Jun 3, 2021

danieleades commented Sep 15, 2021

skovhus left a comment

Choose a reason for hiding this comment

danieleades commented Sep 26, 2021

danieleades commented Sep 26, 2021

neersighted left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 29, 2024

danieleades May 9, 2021 •

edited

Loading