search

MacPython / openblas-libs

BSD 2-Clause "Simplified" License
9 stars 13 forks source link

FIX: Keep the filenames of verified wheels unchanged #140

Closed matthewfeickert closed 9 months ago

matthewfeickert commented 9 months ago

Reverts the portion of PR https://github.com/MacPython/openblas-libs/pull/135 that changes the filename of the wheels.

Resolves https://github.com/numpy/numpy/issues/25849

Once a wheel has been created through a build process and its metadata set and verified it should not be altered as this can invalidate the metadata. A package can have a project name that is - seperated and a wheel filename that is _ seperated and still be fine.

$ git grep "version =" pyproject.toml
pyproject.toml:version = "0.3.26.0.5"
$ cd OpenBLAS/
$ git describe --tags --abbrev=8
v0.3.26
$

c.f. https://github.com/scientific-python/upload-nightly-action/issues/65 for a discussion on why.

(Note that the https://github.com/scientific-python/upload-nightly-action test project used in CI tests uses this convention of project name being test-project and wheel filename being test_package-0.0.1-py3-none-any.whl.)

matthewfeickert commented 9 months ago

@mattip This PR is ready for review (but it also needs approval to run the CI as I'm a first time contributor).

mattip commented 9 months ago

xref https://github.com/Anaconda-Platform/anaconda-client/issues/704 for why this won't work.

mattip commented 9 months ago

This is one alternative to #139

matthewfeickert commented 9 months ago

@mattip To try to answer https://github.com/scientific-python/upload-nightly-action/issues/65#issuecomment-1951833659 here:

The reason why this is amending PR https://github.com/MacPython/openblas-libs/pull/135 is because that PR did two things: Change the project name ( :+1: ) and then also manually change the (edit: project name in the) wheel filename (which generically breaks any wheel). This PR is just reverting that second breaking change.

https://github.com/Anaconda-Platform/anaconda-client/issues/704 is irrelevant to this problem. Anytime you change the project name in a wheel filename manually after building it you've broken it. I've already shown this in https://github.com/scientific-python/upload-nightly-action/issues/65#issuecomment-1951735277 but let's take another example just to show it is a generically invalid thing to do.

I'll use scikit-image as an example, but any wheel works.

$ python -m pip download --no-deps scikit-image
Collecting scikit-image
  Downloading scikit_image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (13 kB)
Downloading scikit_image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (14.7 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 14.7/14.7 MB 12.9 MB/s eta 0:00:00
Saved /scikit_image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Successfully downloaded scikit-image

we'll now rename it in the same way that you did in PR https://github.com/MacPython/openblas-libs/pull/135 (which is reverted here)

$ mv scikit_image*.whl scikit-image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl

when we try to install it we now see it is broken as we've invalidated the metadata

$ python -m pip install --upgrade ./scikit-image-*.whl
Processing /scikit-image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Discarding file:///scikit-image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: Requested scikit==image from file:///scikit-image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl has inconsistent name: expected 'scikit', but metadata has 'scikit-image'
ERROR: Could not find a version that satisfies the requirement scikit (unavailable) (from versions: none)
ERROR: No matching distribution found for scikit (unavailable)

Reverting this breaking naming change gives you a working wheel again.

$ mv scikit-image*.whl scikit_image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
$ python -m pip install --upgrade ./scikit_image-0.22.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
...
Successfully installed imageio-2.34.0 lazy_loader-0.3 networkx-3.2.1 numpy-1.26.4 packaging-23.2 pillow-10.2.0 scikit-image-0.22.0 scipy-1.12.0 tifffile-2024.2.12

This is the problem that you're facing and it has nothing to do with any uploader.

As I've shown in https://github.com/MacPython/openblas-libs/pull/140#discussion_r1494115782 having a project name be - seperated and a wheel filename be _ seperated is a non-issue and both twine and anaconda upload will create projects from the project name.

Here's additional examples of anaconda upload doing the expected thing:

+ anaconda --token '*** ' upload --force --user scientific-python-nightly-wheels --label main dist/scikit_image-0.23.0rc0.dev0-cp310-cp310-macosx_10_9_x86_64.whl
...
Uploading wheels to anaconda.org...
Using Anaconda API: https://api.anaconda.org/
Using "scientific-python-nightly-wheels" as upload username
Processing "dist/scikit_image-0.23.0rc0.dev0-cp310-cp310-macosx_10_9_x86_64.whl"
Detecting file type...
File type is "Standard Python"
Extracting standard python attributes for upload
Creating package "scikit-image"
Creating release "0.23.0rc0.dev0"
Uploading file "scientific-python-nightly-wheels/scikit-image/0.23.0rc0.dev0/scikit_image-0.23.0rc0.dev0-cp310-cp310-macosx_10_9_x86_64.whl"
Warning:  Distribution "scikit_image-0.23.0rc0.dev0-cp310-cp310-macosx_10_9_x86_64.whl" already exists. Removing.

  0%|          | 0.00/13.3M [00:00<?, ?B/s]
 19%|█▉        | 2.58M/13.3M [00:00<00:00, 27.0MB/s]
13.3MB [00:00, 39.5MB/s]                            
Upload complete
...
standard python located at:
  https://anaconda.org/scientific-python-nightly-wheels/scikit-image

Hopefully this clears things up, but if you have questions on anything please let me know. I can try to provide either additional examples or point to specific bits of code.

mattip commented 9 months ago

OK then we are an impass. The $PYTHON -m pip wheel -w dist -vv . command is building a wheel with a -

matthewfeickert commented 9 months ago

Ah, here's a stronger demonstration: Here I download your wheel with the broken filename, fix the filename by reverting it to the project name in the filename the wheel had at creation, and then upload it to the scikit-hep-nightly-wheels org that I control with anaconda upload, and then install it from the index with no problems:

(base) mambauser@5b63c11e112c:/tmp$ curl -sLO https://anaconda.org/scientific-python-nightly-wheels/scipy-openblas32/0.3.26.199/download/scipy-openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
(base) mambauser@5b63c11e112c:/tmp$ mv scipy-openblas32*.whl scipy_openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
(base) mambauser@5b63c11e112c:/tmp$ anaconda --token *** upload --force --user scikit-hep-nightly-wheels --label main scipy_openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 
Using Anaconda API: https://api.anaconda.org
Using "scikit-hep-nightly-wheels" as upload username
Processing "scipy_openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl"
Detecting file type...
File type is "Standard Python"
Extracting standard python attributes for upload
Creating package "scipy-openblas32"
Creating release "0.3.26.199"
Uploading file "scikit-hep-nightly-wheels/scipy-openblas32/0.3.26.199/scipy_openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl"
10.8MB [00:04, 2.56MB/s]                                                                                                                                                                                           
Upload complete

standard python located at:
  https://anaconda.org/scikit-hep-nightly-wheels/scipy-openblas32

(base) mambauser@5b63c11e112c:/tmp$ python -m pip install --index-url https://pypi.anaconda.org/scikit-hep-nightly-wheels/simple scipy-openblas32
Looking in indexes: https://pypi.anaconda.org/scikit-hep-nightly-wheels/simple
Collecting scipy-openblas32
  Downloading https://pypi.anaconda.org/scikit-hep-nightly-wheels/simple/scipy-openblas32/0.3.26.199/scipy_openblas32-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (11.3 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 11.3/11.3 MB 1.7 MB/s eta 0:00:00
Installing collected packages: scipy-openblas32
Successfully installed scipy-openblas32-0.3.26.199
(base) mambauser@5b63c11e112c:/tmp$

image

matthewfeickert commented 9 months ago

OK then we are an impass. The $PYTHON -m pip wheel -w dist -vv . command is building a wheel with a -

@mattip Can you explain more? The build logs (https://github.com/MacPython/openblas-libs/actions/runs/7945503880/job/21692242413#step:8:91) show properly named scipy_openblas32 wheels

+ rm -rf 'dist/*'
+ python3.9 -m pip wheel -w dist -vv .
...
  Building wheel for scipy-openblas32 (pyproject.toml): finished with status 'done'
  Created wheel for scipy-openblas32: filename=scipy_openblas32-0.3.26.199-py3-none-any.whl size=10293919 sha256=2da50374e88fd1643442bc577dde82e271976cff7c4f78fce1517f0922b12eb8
  Stored in directory: /tmp/pip-ephem-wheel-cache-crkxz5g5/wheels/48/8d/83/a2c487e10f8c7b94e06f9fd4a207c7ad7a82214cc3fcc75c33
Successfully built scipy-openblas32
Remote version of pip: 24.0
Local version of pip:  24.0
Was pip installed by pip? True
Removed build tracker: '/tmp/pip-build-tracker-bty2k6ao'

I can also download the artifacts from that build (wheels-ubuntu-latest-x86_64-1-manylinux: https://github.com/MacPython/openblas-libs/actions/runs/7945503880/artifacts/1253961897) where I find a correctly named (filename starts with scipy_openblas64) wheel:

$ unzip wheels-ubuntu-latest-x86_64-1-manylinux.zip 
Archive:  wheels-ubuntu-latest-x86_64-1-manylinux.zip
  inflating: scipy_openblas64-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
$ python -m pip install --upgrade ./scipy_openblas64-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 
Processing /scipy_openblas64-0.3.26.199-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Installing collected packages: scipy-openblas64
Successfully installed scipy-openblas64-0.3.26.199

Where are you seeing these wheels generated with a filename that starts with scipy-openblas32?

rgommers commented 9 months ago

and then also manually change the wheel filename (which generically breaks any wheel).

Quick note: this may be true for the project name part of a wheel (I'm not sure, but it seems plausible indeed), but I'll note that it's not true in general. E.g., changing a platform tag like macosx_11 to macosx_12 is safe and not uncommon.

Thanks for working through this together 🙏🏼

matthewfeickert commented 9 months ago

Quick note: this may be true for the project name part of a wheel (I'm not sure, but it seems plausible indeed), but I'll note that it's not true in general. E.g., changing a platform tag like macosx_11 to macosx_12 is safe and not uncommon.

Yes, very true. I remembered this a few mintues after I originally posted "wheel name" and changed my comment to use "wheel filename" specifically because of this point, but I should have specified here that I mean the project name part.

matthewfeickert commented 9 months ago

@mattip After reading https://github.com/MacPython/openblas-libs/pull/140#issuecomment-1952804212 and https://github.com/MacPython/openblas-libs/pull/140#issuecomment-1952790915 do you agree with my assessment? If so, can you please approve the CI to run on this PR and review it?

mattip commented 9 months ago

Let's merge this, since the uploads will only happen once this is merged.

matthewfeickert commented 9 months ago

Okay, thanks very much. It will be interesting to see if I missed something that matters "in the wild" but if so, this is an easy revert. :+1:

matthewfeickert commented 9 months ago

Looks good! (https://github.com/MacPython/openblas-libs/actions/runs/7995823393/job/21837003098#step:12:184)

+ echo 'Uploading OpenBLAS v0.3.26 to anaconda.org staging:'
+ anaconda -t *** upload --no-progress --force -u scientific-python-nightly-wheels dist/scipy_openblas32-0.3.26.0.5-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
~/work/openblas-libs/openblas-libs
Uploading OpenBLAS v0.3.26 to anaconda.org staging:
Using Anaconda API: https://api.anaconda.org/
Using "scientific-python-nightly-wheels" as upload username
Processing "dist/scipy_openblas32-0.3.26.0.5-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl"
Detecting file type...
File type is "Standard Python"
Extracting standard python attributes for upload
Creating package "scipy-openblas32"
Creating release "0.3.26.0.5"
Uploading file "scientific-python-nightly-wheels/scipy-openblas32/0.3.26.0.5/scipy_openblas32-0.3.26.0.5-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl"

  0%|          | 0.00/10.8M [00:00<?, ?B/s]
  0%|          | 8.00k/10.8M [00:00<06:14, 30.1kB/s]
  1%|          | 96.0k/10.8M [00:00<00:35, 317kB/s] 
  2%|▏         | 256k/10.8M [00:00<00:14, 737kB/s] 
  6%|▌         | 632k/10.8M [00:00<00:06, 1.65MB/s]
 13%|█▎        | 1.37M/10.8M [00:00<00:02, 3.51MB/s]
 32%|███▏      | 3.48M/10.8M [00:00<00:00, 8.71MB/s]
 45%|████▍     | 4.80M/10.8M [00:00<00:00, 10.2MB/s]
 80%|████████  | 8.65M/10.8M [00:01<00:00, 18.4MB/s]
10.8MB [00:01, 8.93MB/s]                            
Upload complete

standard python located at:
  https://anaconda.org/scientific-python-nightly-wheels/scipy-openblas32

image

matthewfeickert commented 9 months ago

Though one strange follow up: https://github.com/numpy/numpy/issues/25849#issuecomment-1958120847

mattip commented 9 months ago

This did not work. 

$ pip install -i https://anaconda.org/scientific-python-nightly-wheels/simple "scipy-openblas32"                                      
Looking in indexes: https://anaconda.org/scientific-python-nightly-wheels                                                                                              
ERROR: Could not find a version that satisfies the requirement scipy-openblas32 (from versions: none)                                                                  
ERROR: No matching distribution found for scipy-openblas32
mattip commented 9 months ago

However, downloading the wheel, copying it to a local path, and asking pip to look there works

$mkdir openblas
$ cp ~/Downloads/scipy_openblas32-0.3.26.0.5-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl openblas
$ pip install -f openblas scipy-openblas32
Looking in links: openblas
Processing ./openblas/scipy_openblas32-0.3.26.0.5-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Installing collected packages: scipy-openblas32
Successfully installed scipy-openblas32-0.3.26.0.5
matthewfeickert commented 9 months ago

@mattip, as mentioned above c.f. https://github.com/numpy/numpy/issues/25849#issuecomment-1958120847