Skip to content

Update type hints of PEP440Version class#270

Merged
neersighted merged 4 commits into
masterfrom
unknown repository
Feb 28, 2022
Merged

Update type hints of PEP440Version class#270
neersighted merged 4 commits into
masterfrom
unknown repository

Conversation

@NiklasRosenstein
Copy link
Copy Markdown
Contributor

@NiklasRosenstein NiklasRosenstein commented Jan 24, 2022

This change allows Mypy to infer that methods like parse() or replace() on a subclass of PEP440Version return the same type and not falling back to the PEP440Version base type.

Example:

from poetry.core.semver.version import Version

version = Version.parse('1.0.0')
next_version = version.next_major()

reveal_type(version)  # poetry.core.semver.version.Version
reveal_type(next_version)  # poetry.core.semver.version.Version  (used to be PEP440Version)

Note that poetry-core can only be used with Mypy after merging #269 anyway.

Comment thread src/poetry/core/version/pep440/version.py Outdated
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@danieleades
Copy link
Copy Markdown
Contributor

this seems slightly surprising to me. I guess i've not seen this construction before. I would have expected to add typing appropriate to the derived classes by specialising the derived class signatures, not the base class signatures. adding type annotations to self and cls is a little jarring

@dimbleby
Copy link
Copy Markdown
Contributor

Pretty annoying to have to implement derived methods just for the sake of the typing, though?

The link is hidden in a now-resolved thread but https://mypy.readthedocs.io/en/stable/generics.html#generic-methods-and-generic-self shows more or less exactly this, albeit with a warning that this is 'experimental' meaning that mypy might say wrong things.

@danieleades
Copy link
Copy Markdown
Contributor

Pretty annoying to have to implement derived methods just for the sake of the typing, though?

absolutely. I don't have any particular issue with this, just not something i've seen in the wild.
Python really needs something like Rust's Self type

@NiklasRosenstein
Copy link
Copy Markdown
Contributor Author

@danieleades A Self type is coming in Python 3.11 (PEP 673)

@dimbleby dimbleby mentioned this pull request Feb 9, 2022
@neersighted neersighted merged commit c5f4cda into python-poetry:master Feb 28, 2022
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.

4 participants