[FIX] Default to using scipy v1.8.0+ API #646

matthewfeickert · 2024-06-25T16:56:24Z

Types of changes

As most users will be installing the latest scipy release when they install POT, default to using the scipy v1.8.0+ API and catch the ModuleNotFoundError in the rare case in which scipy v1.7.3 or older is required. (scipy/scipy#14966 is in scipy v1.8.0+)

In all situations,import from the scipy.optimize.{_}linesearch API as the newest version of scipy that supports Python 3.7 is v1.7.3 which does not support from scipy.optimize import scalar_search_armijo, just as scipy v1.14.0+ does not.

Motivation and context / Related issue

Amends PR #642

How has this been tested (if it applies)

$ docker run --rm -ti python:3.8 /bin/bash
root@62c4a7db4ec9:/# python -m pip --quiet install uv
root@62c4a7db4ec9:/# uv venv
Using Python 3.8.19 interpreter at: usr/local/bin/python3
Creating virtualenv at: .venv
root@62c4a7db4ec9:/# . .venv/bin/activate
(.venv) root@62c4a7db4ec9:/# uv pip install 'scipy==1.8.0'
Resolved 2 packages in 327ms
Prepared 2 packages in 1.10s
Installed 2 packages in 27ms
 + numpy==1.24.4
 + scipy==1.8.0
(.venv) root@62c4a7db4ec9:/# python -c 'from scipy.optimize._linesearch import scalar_search_armijo; print(scalar_search_armijo)'
<function scalar_search_armijo at 0x7536b52294c0>
(.venv) root@62c4a7db4ec9:/#

PR checklist

I have read the CONTRIBUTING document.
The documentation is up-to-date with the changes I made (check build artifacts).
All tests passed, and additional code has been covered with new tests.
- Codecov is giving 0% coverage to the diff, but that is only because the ImportError raising branch isn't ever being taken as the version of scipy to raise it is old.
[N/A] I have added the PR and Issue fix to the RELEASES.md file.
- This doesn't' change the logic from PR [Fix] import linesearch for modern scipy #642 so no need to update the release notes(?)

codecov · 2024-06-25T17:15:44Z

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.65%. Comparing base (14bbebf) to head (56efb61).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #646      +/-   ##
==========================================
- Coverage   96.66%   96.65%   -0.02%     
==========================================
  Files          85       85              
  Lines       16936    16936              
==========================================
- Hits        16372    16370       -2     
- Misses        564      566       +2

matthewfeickert · 2024-06-25T17:22:21Z

CI is passing, so this is now ready for review.

matthewfeickert · 2024-06-25T20:50:16Z

This actually ends up being required to not break Python 3.7:

POT/setup.py

Line 87 in 14bbebf

python_requires=">=3.7",

$ docker run --rm -ti python:3.7 /bin/bash
root@1c521122a4c7:/# python -m venv venv && . venv/bin/activate
(venv) root@1c521122a4c7:/# python -m pip --quiet install --upgrade pip setuptools wheel
(venv) root@1c521122a4c7:/# python -m pip install --upgrade scipy
Collecting scipy
  Downloading scipy-1.7.3-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl.metadata (2.2 kB)
Collecting numpy<1.23.0,>=1.16.5 (from scipy)
  Downloading numpy-1.21.6-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl.metadata (2.1 kB)
Downloading scipy-1.7.3-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (38.1 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 38.1/38.1 MB 45.2 MB/s eta 0:00:00
Downloading numpy-1.21.6-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (15.7 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 15.7/15.7 MB 53.4 MB/s eta 0:00:00
Installing collected packages: numpy, scipy
Successfully installed numpy-1.21.6 scipy-1.7.3
(venv) root@1c521122a4c7:/# apt update && apt install -y vi
(venv) root@1c521122a4c7:/# cat fail.py 
# https://github.com/PythonOT/POT/blob/14bbebf1b6c5a3acef91e07247388945fd7a9014/ot/optim.py#L19-L22
try:
    from scipy.optimize import scalar_search_armijo
except ImportError:
    from scipy.optimize._linesearch import scalar_search_armijo
print(scalar_search_armijo)
(venv) root@1c521122a4c7:/# python fail.py 
Traceback (most recent call last):
  File "fail.py", line 3, in <module>
    from scipy.optimize import scalar_search_armijo
ImportError: cannot import name 'scalar_search_armijo' from 'scipy.optimize' (/venv/lib/python3.7/site-packages/scipy/optimize/__init__.py)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "fail.py", line 5, in <module>
    from scipy.optimize._linesearch import scalar_search_armijo
ModuleNotFoundError: No module named 'scipy.optimize._linesearch'
(venv) root@1c521122a4c7:/# cat pass.py 
# https://github.com/PythonOT/POT/pull/646
try:
    from scipy.optimize._linesearch import scalar_search_armijo
except ModuleNotFoundError:
    from scipy.optimize.linesearch import scalar_search_armijo
print(scalar_search_armijo)
(venv) root@1c521122a4c7:/# python pass.py 
<function scalar_search_armijo at 0x7dd6dc6767a0>
(venv) root@1c521122a4c7:/#

fail.py.txt

pass.py.txt

* As most users will be installing the latest scipy release when they install POT, default to using the scipy v1.8.0+ API and catch the ModuleNotFoundError in the rare case in which scipy v1.7.3 or older is required, such as Python 3.7. * Always import from the scipy.optimize.{_}linesearch API as the newest version of scipy that supports Python 3.7 is v1.7.3 which does not support 'from scipy.optimize import scalar_search_armijo', just as scipy v1.14.0+ does not.

matthewfeickert · 2024-06-26T03:24:00Z

ot/optim.py

        from scipy.optimize._linesearch import scalar_search_armijo
+    except ModuleNotFoundError:
+        # scipy<1.8.0


This comment is added only to make it easier to know when this try except block can be removed when the scipy install_requires lower bound gets raised eventually.

matthewfeickert force-pushed the mnt/default-to-modern-api branch from 4aa59f2 to 22f0282 Compare June 25, 2024 20:38

matthewfeickert changed the title ~~[MNT] Default to using scipy v1.8.0+ API~~ [FIX] Default to using scipy v1.8.0+ API Jun 25, 2024

matthewfeickert force-pushed the mnt/default-to-modern-api branch from 22f0282 to 1092d2d Compare June 25, 2024 20:56

matthewfeickert force-pushed the mnt/default-to-modern-api branch from 1092d2d to 56efb61 Compare June 26, 2024 03:22

matthewfeickert commented Jun 26, 2024

View reviewed changes

rflamary merged commit 5c9c70a into PythonOT:master Jun 26, 2024
17 of 18 checks passed

matthewfeickert deleted the mnt/default-to-modern-api branch June 26, 2024 06:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] Default to using scipy v1.8.0+ API #646

[FIX] Default to using scipy v1.8.0+ API #646

matthewfeickert commented Jun 25, 2024 •

edited

Loading

codecov bot commented Jun 25, 2024 •

edited

Loading

matthewfeickert commented Jun 25, 2024 •

edited

Loading

matthewfeickert commented Jun 25, 2024 •

edited

Loading

matthewfeickert Jun 26, 2024 •

edited

Loading

[FIX] Default to using scipy v1.8.0+ API #646

[FIX] Default to using scipy v1.8.0+ API #646

Conversation

matthewfeickert commented Jun 25, 2024 • edited Loading

Types of changes

Motivation and context / Related issue

How has this been tested (if it applies)

PR checklist

codecov bot commented Jun 25, 2024 • edited Loading

Codecov Report

matthewfeickert commented Jun 25, 2024 • edited Loading

matthewfeickert commented Jun 25, 2024 • edited Loading

matthewfeickert Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

matthewfeickert commented Jun 25, 2024 •

edited

Loading

codecov bot commented Jun 25, 2024 •

edited

Loading

matthewfeickert commented Jun 25, 2024 •

edited

Loading

matthewfeickert commented Jun 25, 2024 •

edited

Loading

matthewfeickert Jun 26, 2024 •

edited

Loading