-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
mypy: adapt type hints to change in poetry-core#442 #6163
mypy: adapt type hints to change in poetry-core#442 #6163
Conversation
@@ -43,7 +44,7 @@ def get_extra_package_names( | |||
# keep record of packages seen during recursion in order to avoid recursion error | |||
seen_package_names = set() | |||
|
|||
def _extra_packages(package_names: Iterable[str]) -> Iterator[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def _extra_packages(
package_names: Iterable[NormalizedName],
) -> Iterator[NormalizedName]:
and remove the redundant canonicalize_name()
at line 52
src/poetry/utils/extras.py
Outdated
from poetry.core.packages.package import Package | ||
|
||
|
||
def get_extra_package_names( | ||
packages: Sequence[Package], | ||
extras: Mapping[str, list[str]], | ||
extras: Mapping[str, list[NormalizedName]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we believe this change, then the call to canonicalize_name
a few lines later is redundant and should be removed
I'm not completely sure that I do believe it: some callers populate this parameter by reading from the locker: is lock_data.get("extras", {})
definitely returning NormalizedName
s?
I think the answer is that it is if the lockfile was written by a sufficiently recent poetry, possibly it isn't otherwise. This isn't normalized, even in recent poetry lockfiles: so this type annotation is faulty.
(I experimented with adding an extra like this
[tool.poetry.dependencies]
"zope.interface" = { version = ">=1", optional = true }
[tool.poetry.extras]
foo = ["zope.interface"]
and the unnormalized zope.interface
is carried through to the lockfile)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, poetry 1.2 should be able to read lock files generated by poetry 1.1. Thus, I think we should keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both poetry 1.1 and 1.2 write un-normalized strings to the lockfile and so the annotation is flat wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, now I see.
ef9a76a
to
e7d562e
Compare
e7d562e
to
176f89a
Compare
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. |
Necessary changes when updating poetry-core to a version containing python-poetry/poetry-core#442