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

Allow scipy>=1.13 #270

Merged
merged 3 commits into from
Jun 10, 2024
Merged

Allow scipy>=1.13 #270

merged 3 commits into from
Jun 10, 2024

Conversation

kalekundert
Copy link
Contributor

I looked into #269 a little more closely, and I found that the test suite passes with scipy 1.13, but not with 1.12. I think this is due to the fact that 1.13 includes a fix for scipy/scipy#16792.
Given this, I adjusted the scipy dependency version specifier to allow <=1.9 or >=1.13. (More specifically, I excluded versions 1.10, 1.11, and 1.12.) This allows pymanopt to be successfully installed for python 3.12.

I also configured GitHub Actions to run the test suite on the most recent version of python, which should hopefully catch errors like this sooner in the future.

Old versions of setuptools depend on the `pkgutil` standard library
module, which is slated to be removed.
@NicolasBoumal NicolasBoumal requested a review from nkoep May 15, 2024 14:24
Copy link
Contributor

@antoinecollas antoinecollas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kale,
Thank you a lot for contributing to pymanopt!

@@ -15,6 +15,9 @@ jobs:
- "3.8"
- "3.9"
- "3.10"
- "3.11"
- "3.12"
- "3.x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does 3.x mean, and why do you need it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.x does not pass. Can you remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.x means "the most recent version of python". Right now, 3.x is specifically 3.12.3, which is the same as 3.12. So getting rid of 3.x wouldn't solve the problem, because the 3.12 test would fail for the same reason.

I looked into the failure, though, and I'm pretty sure it was caused by a too-old version of setuptools.
Python 3.12 requires setuptools>=66.1.0 (see pypa/setuptools#3685), but the version of setuptools used for CI was pinned to 65.6.3. Note that this is actually the same problem I tried to address in d6bbca6, but in that commit I only updated the setuptools version used to install pymanopt itself. I didn't notice that there was another pin specifically for CI. I just pushed a commit that updates this pin; hopefully that will work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for having looked at this!

@coveralls
Copy link

Coverage Status

coverage: 88.102% (+0.04%) from 88.066%
when pulling 727956f on kalekundert:fix-py312
into 5e38c0f on pymanopt:master.

@NicolasBoumal NicolasBoumal requested review from NicolasBoumal and removed request for nkoep June 10, 2024 13:27
@NicolasBoumal NicolasBoumal merged commit 1de3b6f into pymanopt:master Jun 10, 2024
15 checks passed
@kalekundert kalekundert deleted the fix-py312 branch June 10, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants