Skip to content

Use packaging.version for correct version parsing#4330

Closed
tjstum wants to merge 3 commits intofacebookresearch:mainfrom
tjstum:main
Closed

Use packaging.version for correct version parsing#4330
tjstum wants to merge 3 commits intofacebookresearch:mainfrom
tjstum:main

Conversation

@tjstum
Copy link
Contributor

@tjstum tjstum commented May 5, 2025

If you use a version of numpy that has non-numeric components to the version number (e.g. a local version identifier), faiss will not import because the hand-rolled version parsing expects only digits:

$ /venv/bin/python -c 'import numpy; print(numpy.__version__)'
2.0.2+hrt0
$ /venv/bin/python -c 'import faiss'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/venv/lib/python3.12/site-packages/faiss/__init__.py", line 17, in <module>
    from .loader import *
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 91, in <module>
    instruction_sets = supported_instruction_sets()
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 47, in supported_instruction_sets
    if Version(numpy.__version__) >= Version("1.19"):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 13, in Version
    return [int(x) for x in v.split('.')]
            ^^^^^^
ValueError: invalid literal for int() with base 10: '2+hrt0'

This project depends on packaging, but the use was removed in #3807 (though the metadata wasn't). This restores the correct parsing of Version. If you'd prefer not to add this dependency, what is your suggestion for correctly parsing the numpy version (also, the install_requires line should be updated)

@facebook-github-bot
Copy link
Contributor

@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@junjieqi merged this pull request in 3d7659e.

samanthawaters8882michaeldonovan added a commit to samanthawaters8882michaeldonovan/faiss that referenced this pull request Oct 12, 2025
Summary:
If you use a version of numpy that has non-numeric components to the version number (e.g. a [local version identifier](https://peps.python.org/pep-0440/#local-version-identifiers)), `faiss` will not import because the hand-rolled version parsing expects only digits:
```
$ /venv/bin/python -c 'import numpy; print(numpy.__version__)'
2.0.2+hrt0
$ /venv/bin/python -c 'import faiss'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/venv/lib/python3.12/site-packages/faiss/__init__.py", line 17, in <module>
    from .loader import *
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 91, in <module>
    instruction_sets = supported_instruction_sets()
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 47, in supported_instruction_sets
    if Version(numpy.__version__) >= Version("1.19"):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/faiss/loader.py", line 13, in Version
    return [int(x) for x in v.split('.')]
            ^^^^^^
ValueError: invalid literal for int() with base 10: '2+hrt0'
```

This project depends on `packaging`, but the use was removed in facebookresearch/faiss#3807 (though the metadata wasn't). This restores the correct parsing of `Version`. If you'd prefer not to add this dependency, what is your suggestion for correctly parsing the numpy version (also, the `install_requires` line should be updated)

Pull Request resolved: facebookresearch/faiss#4330

Reviewed By: mnorris11

Differential Revision: D74592146

Pulled By: junjieqi

fbshipit-source-id: 258c5b499d356040ff6e373b2b635efa355c50ea
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants