Skip to content
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

pip 21 rejects wheels with local version labels #9628

Open
takluyver opened this issue Feb 19, 2021 · 8 comments
Open

pip 21 rejects wheels with local version labels #9628

takluyver opened this issue Feb 19, 2021 · 8 comments
Labels
PEP implementation Involves some PEP

Comments

@takluyver
Copy link
Member

PR #9320 added some metadata validation of wheels built through pip. People using local version identifiers (e.g. 0.1+deadbeef) reported bugs in Flit because it builds wheels which pip rejects based on the filename, like this:

Building wheels for collected packages: testpkg
  Building wheel for testpkg (PEP 517) ... done
  Created wheel for testpkg: filename=testpkg-0.6.1.dev5_ga652c20-py3-none-any.whl size=1181 sha256=cb8856b9035b1cf8021533a8ce7a07f02402c942603ce53e17a5ceea7e9cd787
  Stored in directory: /home/sturm/.cache/pip/wheels/34/c8/42/38155569da3179727ba8f011d468b6a0e6f789986ec22ae3d0
  WARNING: Built wheel for testpkg is invalid: Wheel has unexpected file name: expected '0.6.1.dev5+ga652c20', got '0.6.1.dev5-ga652c20'
Failed to build testpkg

I believe Flit is complying with the spec, specifically PEP 427's section on escaping in the wheel filename:

Each component of the filename is escaped by replacing runs of non-alphanumeric characters with an underscore _:
re.sub("[^\w\d.]+", "_", distribution, re.UNICODE)

I understand that to mean that a 0.1+deadbeef version number should make a .whl filename containing -0.1_deadbeef-.Pip appears to reject that, and I believe it's pip that has this wrong. PyPUG still says that PEP 427 is the sole specification for the wheel format.

If the outcome is that the spec changes to match what pip does, that's fine, and I will fix Flit accordingly. But I don't want to assume that's the case and merge something that goes against the current spec.

@uranusjr
Copy link
Member

uranusjr commented Feb 19, 2021

There was actually a discussion on this a while ago. The problem with escaping + to _ is that the escaped value cannot be converted back to its original value, thus rendered useless. Quoting pypa/wheel#269 (comment):

That part of the spec is probably a mistake where it says "all components of the filename..."

Given Daniel is the author of the wheel format, pip is actually doing it right; PEP 427 is wrong (and should be fixed).

@uranusjr uranusjr added the PEP implementation Involves some PEP label Feb 19, 2021
@uranusjr
Copy link
Member

Cross-linking python/peps#1824.

@ivirshup
Copy link

For backwards compatibility, shouldn't wheels which are built following the current PEP-427 mangling and 440 version specs be installable? My feeling is that a wheel made following current PEP specs should be allowed, regardless of whether the spec might be amended. Once amended, isn't some period of backward compatibility still expected?

My practical concern is: I'd like to keep pip up to date on my projects CI. Using flit for our build system requires that we pin pip to an older version until this is resolved.

@uranusjr
Copy link
Member

uranusjr commented Feb 21, 2021

The problem is, if both + and - are escaped into _, pip cannot know if 0.1_2 came from 0.1+2 or 0.1-2 (both are valid versions) and can't even recognise the file is a match or not, let along allowing it.

@ivirshup
Copy link

The version should still be resolvable from the name in the metadata, just not the filename, right? The metadata wouldn't have been mangled (at least that's my understanding).

Here's the last few lines of what I see if I try pip installing my project using flit as the build tool:

  WARNING: Built wheel for scanpy is invalid: Wheel has unexpected file name: expected '1.7.0rc2.dev33+gf953fbc8', got '1.7.0rc2.dev33-gf953fbc8'
Failed to build scanpy
ERROR: Could not build wheels for scanpy which use PEP 517 and cannot be installed directly

It looks to me like the build is failing to install because the metadata doesn't match the filename, not that pip is unable to figure out the version string.

@uranusjr
Copy link
Member

Sure, but how does pip find that file in the first place? Download all files that look like it and check one by one? That decision would probably not be too popular.

@takluyver
Copy link
Member Author

In pypa/flit#383, people are running into this error in a context where pip doesn't need to find the version: running pip install . (via flit install). Pip gets a specific wheel file, and then refuses to install it because of the name.

I understand that local version identifiers (with a +) can't be uploaded to PyPI anyway. Of course, you could still have one of these packages in a private index or folder - but as people only started reporting this after pip added an explicit check, I suspect that's rare. 🙂

@ivirshup
Copy link

Exactly what @takluyver said, there isn't any searching for a package in this use case, as these wheels couldn't be uploaded to PyPI.

flying-sheep added a commit to scverse/scanpy that referenced this issue Mar 1, 2021
flying-sheep added a commit to scverse/anndata that referenced this issue Mar 1, 2021
flying-sheep added a commit to scverse/anndata that referenced this issue Mar 1, 2021
flying-sheep added a commit to scverse/anndata that referenced this issue Mar 1, 2021
Zethson pushed a commit to scverse/scanpy that referenced this issue Mar 15, 2021
ivirshup added a commit to scverse/scanpy that referenced this issue Mar 18, 2021
* add flake8 pre-commit

Signed-off-by: Zethson <[email protected]>

* fix pre-commit

Signed-off-by: Zethson <[email protected]>

* add E402 to flake8 ignore

Signed-off-by: Zethson <[email protected]>

* revert neighbors

Signed-off-by: Zethson <[email protected]>

* fix flake8

Signed-off-by: Zethson <[email protected]>

* address review

Signed-off-by: Zethson <[email protected]>

* fix comment character in .flake8

Signed-off-by: Zethson <[email protected]>

* fix test

Signed-off-by: Zethson <[email protected]>

* black

Signed-off-by: Zethson <[email protected]>

* review round 2

Signed-off-by: Zethson <[email protected]>

* review round 3

Signed-off-by: Zethson <[email protected]>

* readded double comments

Signed-off-by: Zethson <[email protected]>

* Ignoring E262 & reverted comment

Signed-off-by: Zethson <[email protected]>

* using self for obs_tidy

Signed-off-by: Zethson <[email protected]>

* Restore setup.py

* rm call of black test (#1690)

* Fix print_versions for python<3.8 (#1691)

* add codecov so we can have a badge to point to (#1693)

* Attempt server-side search (#1672)

* Fix paga_path (#1047)

Fix paga_path

Co-authored-by: Isaac Virshup <[email protected]>

* Switch to flit

This reverts commit d645790

* add setup.py while leaving it ignored

* Update install instructions

* Circumvent new pip check (see pypa/pip#9628)

* Go back to regular pip (#1702)

* Go back to regular flit

Co-authored-by: Isaac Virshup <[email protected]>

* codecov comment (#1704)

* Use joblib for parallelism in regress_out (#1695)

* Use joblib for parallism in regress_out

* release note

* fix link in release notes

* Add todo for resource test

* Add sparsificiation step before sparse-dependent Scrublet calls (#1707)

* Add sparsificiation step before sparse-dependent Scrublet calls

* Apply sparsification suggestion

Co-authored-by: Isaac Virshup <[email protected]>

* Fix imports

Co-authored-by: Isaac Virshup <[email protected]>

* Fix version on Travis (#1713)

By default, Travis does `git clone --depth=50` which means the version can’t be detected from the git tag.

* `sc.metrics` module (add confusion matrix & Geary's C methods) (#915)

* Add `sc.metrics` with `gearys_c`

Add a module for computing useful metrics. Started off with Geary's C since I'm using it and finding it useful. I've also got a fairly fast way to calculate it worked out.

Unfortunatly my implementation runs into some issues with some global configs set by umap (see lmcinnes/umap#306), so I'm going to see if that can be resolved before changing it.

* Add sc.metrics.confusion_matrix

* Better tests and output for confusion_matrix

* Workaround umap<0.4 and increase numerical stability of gearys_c

* Work around lmcinnes/umap#306 by not
  calling out to kernel function. That code has been kept, but commented
  out.
* Increase numerical stability by casting data to system width. Tests
  were failing due to instability.

* Split up gearys_c tests

* Improved unexpected error message

* gearys_c working again. Sadly, a bit slower

* One option for doc strings

* Simplify implementation to use single dispatch

* release notes

* Fix clipped images in docs (#1717)

* Cleanup normalize_total (#1667)

* Cleanup normalize_total

* Add modification tests and copy kwarg for normalize_total

* Test that 'layers' argument is deprecated

* Added more mutation checks for normalize_total

* release note

* Error message

* deprecate scvi (#1703)

* deprecate scvi

* Update .azure-pipelines.yml

Co-authored-by: Isaac Virshup <[email protected]>

* remove :func: links to scvi in release notes

* remove tildes in front of scvi in release notes

* Update docs/release-notes/1.5.0.rst

Co-authored-by: Michael Jayasuriya <[email protected]>
Co-authored-by: Isaac Virshup <[email protected]>

* updated ecosystem.rst to add triku (#1722)

* Minor addition to contributing docs (#1726)

* Preserve category order when groupby is a list (#1735)

Preserve category order when groupby is a list

* Asymmetrical diverging colormaps and vcenter (#1551)

Add vcenter and norm arguments to plotting functions

* add flake8 pre-commit

Signed-off-by: Zethson <[email protected]>

* add E402 to flake8 ignore

Signed-off-by: Zethson <[email protected]>

* revert neighbors

Signed-off-by: Zethson <[email protected]>

* address review

Signed-off-by: Zethson <[email protected]>

* black

Signed-off-by: Zethson <[email protected]>

* using self for obs_tidy

Signed-off-by: Zethson <[email protected]>

* rebased

Signed-off-by: Zethson <[email protected]>

* rebasing

Signed-off-by: Zethson <[email protected]>

* rebasing

Signed-off-by: Zethson <[email protected]>

* rebasing

Signed-off-by: Zethson <[email protected]>

* add flake8 to dev docs

Signed-off-by: Zethson <[email protected]>

* add autopep8 to pre-commits

Signed-off-by: Zethson <[email protected]>

* add flake8 ignore docs

Signed-off-by: Zethson <[email protected]>

* add exception todos

Signed-off-by: Zethson <[email protected]>

* add ignore directories

Signed-off-by: Zethson <[email protected]>

* reinstated lambdas

Signed-off-by: Zethson <[email protected]>

* fix tests

Signed-off-by: Zethson <[email protected]>

* fix tests

Signed-off-by: Zethson <[email protected]>

* fix tests

Signed-off-by: Zethson <[email protected]>

* fix tests

Signed-off-by: Zethson <[email protected]>

* fix tests

Signed-off-by: Zethson <[email protected]>

* Add E741 to allowed flake8 violations.

Co-authored-by: Isaac Virshup <[email protected]>

* Add F811 flake8 ignore for tests

Co-authored-by: Isaac Virshup <[email protected]>

* Fix mask comparison

Co-authored-by: Isaac Virshup <[email protected]>

* Fix mask comparison

Co-authored-by: Isaac Virshup <[email protected]>

* fix flake8 config file

Signed-off-by: Zethson <[email protected]>

* readded autopep8

Signed-off-by: Zethson <[email protected]>

* import Literal

Signed-off-by: Zethson <[email protected]>

* revert literal import

Signed-off-by: Zethson <[email protected]>

* fix scatterplot pca import

Signed-off-by: Zethson <[email protected]>

* false comparison & unused vars

Signed-off-by: Zethson <[email protected]>

* Add cleaner level determination

Co-authored-by: Isaac Virshup <[email protected]>

* Fix comment formatting

Co-authored-by: Isaac Virshup <[email protected]>

* Add smoother dev documentation

Co-authored-by: Isaac Virshup <[email protected]>

* fix flake8

Signed-off-by: Zethson <[email protected]>

* Readd long comment

Co-authored-by: Isaac Virshup <[email protected]>

* Assuming X as array like

Co-authored-by: Isaac Virshup <[email protected]>

* fix flake8

Signed-off-by: Zethson <[email protected]>

* fix flake8 config

Signed-off-by: Zethson <[email protected]>

* reverted rank_genes

Signed-off-by: Zethson <[email protected]>

* fix disp_mean_bin formatting

Co-authored-by: Isaac Virshup <[email protected]>

* fix formatting

Signed-off-by: Zethson <[email protected]>

* add final todos

Signed-off-by: Zethson <[email protected]>

* boolean checks with is

Signed-off-by: Zethson <[email protected]>

* _dpt formatting

Signed-off-by: Zethson <[email protected]>

* literal fixes

Signed-off-by: Zethson <[email protected]>

* links to leafs

Signed-off-by: Zethson <[email protected]>

* revert paga variable naming

Co-authored-by: Philipp A <[email protected]>
Co-authored-by: Sergei Rybakov <[email protected]>
Co-authored-by: Isaac Virshup <[email protected]>
Co-authored-by: Jonathan Manning <[email protected]>
Co-authored-by: mjayasur <[email protected]>
Co-authored-by: Michael Jayasuriya <[email protected]>
Co-authored-by: Alex M. Ascensión <[email protected]>
Co-authored-by: Gökçen Eraslan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PEP implementation Involves some PEP
Projects
None yet
Development

No branches or pull requests

3 participants