Filter out yanked links from available versions error message #12225
Conversation
ddelange
force-pushed
the
filter-available-versions
branch
from
600e44a to
bceb840
Compare
ddelange
force-pushed
the
filter-available-versions
branch
from
bceb840 to
300ae87
Compare
ddelange
force-pushed
the
filter-available-versions
branch
2 times, most recently
from
a0d9a7d to
7eb7afa
Compare
|
I wonder if this would introduce confusion from the other direction, i.e. pip must have a bug to not find a version that’s listed on PyPI. It may be a good idea to still show the yanked versions, either separately or annotated. |
ddelange
force-pushed
the
filter-available-versions
branch
from
7eb7afa to
604fc3d
Compare
|
@uranusjr I added a conditional |
ddelange
force-pushed
the
filter-available-versions
branch
4 times, most recently
from
66a5ef2 to
ad8da8b
Compare
| yanked_versions = [ | ||
| str(v) | ||
| for v in sorted( | ||
| {c.version for c in cands if (c.link.is_yanked if c.link else False)} | ||
| ) | ||
| ] |
There was a problem hiding this comment.
Maybe we can compare len(versions) and len(cands) and skip building this list if they match (indicating there’re no yanked versions at all)
There was a problem hiding this comment.
Also I kind of wonder if the yanked version can be info instead. That’s just minor and subjective though.
There was a problem hiding this comment.
if I understood @pfmoore correctly, a version can be in both lists simultaneously if only some of the links for this version are yanked.
nothing to worry about for PyPI, but maybe this can be encountered elsewhere in the wild?
There was a problem hiding this comment.
Good point. I’ll accept this as-is and someone else can worry about optimisation later.
There was a problem hiding this comment.
An equivalent single-lookup variation:
is_yanked = {True: set(), False: set()}
for c in cands:
is_yanked[c.link.is_yanked if c.link else False].add(c.version)
versions = sorted(is_yanked[False])
yanked_versions = sorted(is_yanked[True])How about that?
ddelange
force-pushed
the
filter-available-versions
branch
4 times, most recently
from
bf63604 to
711c4b8
Compare
|
|
||
| is_yanked: Dict[bool, Set[CandidateVersion]] = {True: set(), False: set()} | ||
| for c in cands: | ||
| is_yanked[c.link.is_yanked if c.link else False].add(c.version) |
There was a problem hiding this comment.
@sbidoul the same reasoning from #12224 (comment) would apply here
| is_yanked[c.link.is_yanked if c.link else False].add(c.version) | ||
|
|
||
| versions = sorted(str(v) for v in is_yanked[False]) | ||
| yanked_versions = sorted(str(v) for v in is_yanked[True]) |
There was a problem hiding this comment.
We should sort the versions, then convert to str for the sorting to be accurate.
There was a problem hiding this comment.
great catch! could you have a look again?
|
|
||
| is_yanked: Dict[bool, Set[CandidateVersion]] = {True: set(), False: set()} |
There was a problem hiding this comment.
This dict feels a little bit awkward to me. I'd have done sets and a good old if. It might be a matter of personal preference though.
ddelange
force-pushed
the
filter-available-versions
branch
from
21f98c4 to
0818242
Compare
ddelange
force-pushed
the
filter-available-versions
branch
from
0818242 to
510c6ac
Compare
Fixes #11745