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 python 3.12 and scipy 1.12 #37270

Merged
merged 8 commits into from
Feb 13, 2024
Merged

Conversation

tornaria
Copy link
Contributor

@tornaria tornaria commented Feb 9, 2024

This is a minimal PR to allow building with python 3.12, given the
circumstances surrounding #36181.

I've tested this works with system python 3.12 by:

./bootstrap
./configure --disable-doc --disable-editable --enable-system-site-packages
env 'MAKE=make -j36' make
./sage -tp 36 --all
./sage -tp 36 --all --long

It only gave the expected failures in
src/sage/matroids/database_collections.py from #37140.

Since scipy 1.12 works ok after #37123 and it's just a one-liner to
change the required version, I included it here.

No attempt is made at upgrading anything in sage-the-distro.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.

EDIT: rebased on top of #36983 to address reviewer suggestion.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 9, 2024

This is missing the changes from #36983, so this will only work with system python 3.12 that has resurrected distuils.

@tornaria
Copy link
Contributor Author

tornaria commented Feb 9, 2024

This is missing the changes from #36983, so this will only work with system python 3.12 that has resurrected distuils.

I don't have "resurrected distutils" and it works.

@dimpase
Copy link
Member

dimpase commented Feb 9, 2024

we're entering a delightful subject of Python (without Monty) theology.

What's resurrection of distutils?

@tornaria
Copy link
Contributor Author

tornaria commented Feb 9, 2024

I rebased it on top of #36983. But I don't like this workflow of adding "dependent PRs" and it makes this PR harder to review.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 9, 2024

What's resurrection of distutils?

Well, distutils was killed in CPython 3.12.
If you can still import it in a system Python 3.12, it is because it has been resurrected.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 9, 2024

All joking aside, it is of course setuptools that makes its vendored version of distutils (setuptools._distutils) available as distutils, via a complicated mechanism: https://github.com/pypa/setuptools/blob/main/setup.py#L52, which is slated for removal:

@tornaria
Copy link
Contributor Author

tornaria commented Feb 9, 2024

All joking aside: is this good now?

build/pkgs/python3/SPKG.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me. Feel free to set it to "positive review" once the remark by @Jamim is committed.

Co-authored-by: Aliaksei Urbanski <aliaksei.urbanski@gmail.com>
@tornaria
Copy link
Contributor Author

Apologies for the force-push. I replaced the last commit by an identical one except for my email address since github used a incorrect one.

Copy link

Documentation preview for this PR (built with commit cbde6e7; changes) is ready! 🎉

@vbraun vbraun merged commit 75afadb into sagemath:develop Feb 13, 2024
20 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Mar 7, 2024
@tornaria tornaria deleted the python-3.12 branch March 12, 2024 13:50
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 25, 2024
    
This makes it so that `sage_setup` is included in `sagemath_standard`
for convenience and ease of installation.

Having `sage_setup` as a separate distribution leads to chicken-and-egg
problem. For example, this is what happens when I try to build
`sagemath-standard` from sagemath#37270:
```
cd pkgs/sagemath-standard
$ python -m build -s
* Creating venv isolated environment...
* Installing packages in isolated environment... (cypari2 >=2.1.1,
cysignals >=1.10.2, cython >=3.0, != 3.0.3, <4.0, gmpy2 ~=2.1.b999,
jinja2 >=3.0, jupyter_core >=4.6.3, memory_allocator, numpy >=1.19,
pkgconfig, pplpy >=0.8.6, sage-conf ~= 10.3b7, sage-setup ~= 10.3b7,
sage_setup[autogen], setuptools >= 68.1.1, wheel >=0.36.2)

[... snip ...]

ERROR: Could not find a version that satisfies the requirement sage-
setup~=10.3b7 (from versions: 9.4rc1)
ERROR: No matching distribution found for sage-setup~=10.3b7

[...]
```
This is because the version of sage-setup which is available on pypi
requires python <= 3.11. Even if the branch I'm trying to build allows
python 3.12, I cannot use it.

The main idea of this change is that `sagemath-standard` sdists are
self-contained and include the exact matching version of sage-setup. The
self-contained part is good for the pypi sdist, but the matching version
allows for greater flexibility when testing, given that the
implementations of `sagemath-standard/setup.py` and `sage_setup` are so
intrincately connected and really need to match.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37287
Reported by: Gonzalo Tornaría
Reviewer(s): Dima Pasechnik, Tobias Diez
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.

6 participants