sage
sage copied to clipboard
sagemath
Readd init files
There are various issues with using namespace packages because essential dependencies of sage are not fully supporting them. In particular,
- As seen in https://github.com/sagemath/sage/pull/36228 and #36524, cython currently doesn't support namespace packages (it creates the wrong module name, perhaps related to https://github.com/cython/cython/issues/5237). Sagelib (more specifically sage-setup) currently implements an ugly workaround by monkey-patching Cython internals
- Sage's on-the-fly cython compilation is currently completely broken as none of the namespace packages can be used in it (see the changed doctest in this PR)
- There are various issues with pytest, some of the are discussed in https://github.com/sagemath/sage/issues/33826
- importlib.resources doesn't support namespace packages on Python 3.9. https://github.com/sagemath/sage/pull/37344#issuecomment-1951560051
- It is not possible to compile and use a cython file using the standard cython cli. This, however, can be very helpful in certain situations where you want to experiment with different cython options but don't want to wait for a complete recompilation of all cython files. I used this quite often while preparing #36524.
Until these issues are fixed upstream, we revert https://github.com/sagemath/sage/pull/35322 and https://github.com/sagemath/sage/pull/35366 by readding the __init__.py files.
The proposed alternative https://github.com/sagemath/sage/pull/37009 would only provide an (ugly) workaround for the last issue.
:memo: 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.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
:hourglass: Dependencies
This change is not needed in Sage, so why not just keep this workaround in your experimental branch
This change is not needed in Sage
It fixes the issues you see in https://github.com/sagemath/sage/pull/36228. Not sure if you count your PR as being part of Sage or not. This PR also shows that the only reason why its currently working in the develop branch is that sage is monkey patching cython's package discovery.
so why not just keep this workaround in your experimental branch
Because https://github.com/sagemath/sage/pull/36524 is already in a good shape (there is only one compilation issue left), so to ease review of it I started to extract changes in that branch to new PRs.
This change is not needed in Sage
It fixes the issues you see in #36228. Not sure if you count your PR as being part of Sage or not. This PR also shows that the only reason why its currently working in the develop branch is that sage is monkey patching cython's package discovery.
Yes, that is precisely the purpose of our customization of Cython.
Obviously, #36228 is not part of Sage. It is a draft PR that investigates what the status of upstream Cython namespace package support is. We cannot yet switch to it.
We cannot yet switch to it.
Should work with the changes of this PR. Give it a try: https://github.com/mkoeppe/sage/pull/20
Obviously, when you replace namespace packages by ordinary packages, then namespace package support is not needed. What's there to try out.
But we need the namespace packages for the modularization -- which is why we removed the init files.
So why not instead of calling cython from meson, call a custom script that uses our customization.
Obviously, when you replace namespace packages by ordinary packages, then namespace package support is not needed. What's there to try out.
Matthias, it's not my fault that Cython's namespace package support has bugs. So what's the problem with not using it (for now, until it's ready)?
But we need the namespace packages for the modularization -- which is why we removed the init files.
It runs perfectly fine right now. If additional modularized distributions need namespace support, just don't install the init files (they are needed for compilation only). (Or design around the cython limitation for now and don't use namespace packages.)
So why not instead of calling
cythonfrom meson, call a custom script that uses our customization.
I'm not calling cython myself. It's all handled internally by meson, see https://mesonbuild.com/Python-module.html#extension_module.
why not instead of calling
cythonfrom meson, call a custom script that uses our customization.I'm not calling cython myself. It's all handled internally by meson, see https://mesonbuild.com/Python-module.html#extension_module.
Without doubt, trivial to customize.
Cython's namespace package support has bugs. So what's the problem with not using it (for now, until it's ready)?
Once again, Sage is not affected by this bug because we have a working customization of Cython.
Cython's namespace package support has bugs. So what's the problem with not using it (for now, until it's ready)?
Once again, Sage is not affected by this bug because we have a working customization of Cython.
What's the point of this posturing? Do you just want to make my work on https://github.com/sagemath/sage/pull/36524 harder?
Clearly there are bugs in Cython. The options (brought forward so far) are either a) the current approach of monkey patching or b) just not using namespace modules during compilation (i.e. this PR here). Both are workarounds for the same Cython bug. Option a) does not work (easily) for https://github.com/sagemath/sage/pull/36524, while b) does work well. So why not go with b)?
Unfortunately @mkoeppe prefers quick "clever" workarounds to proper, standard ways to do things; instead of using external packages as everybody does, Sage vendors most everything, and is adding more and more vendored packages at an alarming rate.
What's the point of this posturing? Do you just want to make my work on #36524 harder?
@tobiasdiez I have made clear to you in https://github.com/sagemath/sage/pull/36572#issuecomment-1803044465 that insinuations of this type are unwelcome. I'm referring this to sage-abuse again.
Cython's namespace package support has bugs. So what's the problem with not using it (for now, until it's ready)?
Once again, Sage is not affected by this bug because we have a working customization of Cython.
Could we please have as major goal to stop using "customization" of standard tools? This only adds a lot of friction to making sagemath more modular as in: work in a normal environment with reasonably up to date dependencies (and don't require them to be patched just to suit sagemath).
If sage-the-distro is meant as a "reference distribution" of sagemath, it should strive to ship packages from unpatched, upstream released versions.
This is incompatible with the modularization work and cannot be merged.
any CI failures indicating this?
This is incompatible with the modularization work and cannot be merged.
At the very least I systematically need to touch src/sage/__init__.py because sagemath is broken without this file.
I systematically need to
touch src/sage/__init__.pybecause sagemath is broken without this file.
Because of a system install of Sage components as discussed previously?
Could we please have as major goal to stop using "customization" of standard tools?
Of course, that's a major goal.
- #31543
I systematically need to
touch src/sage/__init__.pybecause sagemath is broken without this file.Because of a system install of Sage components as discussed previously?
Yes, and before you tell me this is "unsupported", it works very well, thank you very much, except for the missing __init__ files (which I think also cause some trouble when compiling cython code external, perhaps due to relative imports that also provoke other issues but the root cause seems to always be the lack of init files).
Maybe your goal is a "modular walled garden".
And concretely for the problem with namespace packages in current Cython, this is:
- #36228
This will need some work with upstream, which would be very valuable. I don't have time for this at the moment, so any help is very welcome.
As I have said in comments above, for Sage there is nothing to fix because we have the customization in place.
Could we please have as major goal to stop using "customization" of standard tools?
Of course, that's a major goal.
* [Meta-ticket: Eliminate use of patches for Python packages #31543](https://github.com/sagemath/sage/issues/31543)
You know, what strikes me the most, is that you, @dimpase and @tobiasdiez (and I?) seem to be all really in a very strong agreement on most of the substantial matters, and you (we?) keep arguing about implementation details instead of focusing on what's important.
Having or not having init files, using or not using namespace packages, are just implementation details. Having feedback on the form "this breaks my workflow, please don't be a sucker" is an asset, not a liability. I keep doing it because I know you care, you listen, and you are able to figure out more robust ways to achieve the goals. I also try to adjust my workflow, often with your help. I think that's more constructive than turning the review labels into a flip-flop.
@tornaria Of course your feedback is always appreciated.
But some "implementation details" matter very much -- because of external constraints, namely the Python "ecosystem". Implicit namespace packages are the standard defined by PEP 420. I am using them because it allows us to separate distribution management from namespace management: I have been able to modularize without renaming hundreds of modules.
Yes, we are on the cutting edge with this extensive use of namespace packages, but the remaining minor troubles that come from this are easy to solve.
@tornaria Of course your feedback is always appreciated.
But some "implementation details" matter very much -- because of external constraints, namely the Python "ecosystem". Implicit namespace packages are the standard defined by PEP 420. I am using them because it allows us to separate distribution management from namespace management: I have been able to modularize without renaming hundreds of modules.
But do we really want to ship the sage library split into pieces? You convinced me before that the real gain is that one can not install e.g. the sagemath-pari distribution, so that pari doesn't have to be installed. But maybe there is another way: ship the sage library as a whole, but activate the "sagemath-pari" part only if pari is available, etc. IOW, keep sage the library as a whole, but make it more "dynamic" on what software it works with. There could still be a "sagemath-pari" pip package, but it won't need to install anything into the sage library but only make sure pari is working.
but activate the "sagemath-pari" part only if pari is available, etc. IOW, keep sage the library as a whole, but make it more "dynamic" on what software it works with.
No, this is incompatible with Python packaging. Functionality is keyed to distribution names.
but activate the "sagemath-pari" part only if pari is available, etc. IOW, keep sage the library as a whole, but make it more "dynamic" on what software it works with.
No, this is incompatible with Python packaging. Functionality is keyed to distribution names.
Why? Python itself is dynamic, and there are tons of python packages with functionality that is disabled or enabled on the presence of another python package.
Python isn't really in a great shape - you can argue that PEP 420 is standard, but neither Cython nor Pytest implemented it.
CPython is taken by "speeduppers", who try to make sure that the COBOL-like tasks important for their employers are fast, submit PEPs which get rejected only to sneak them in via backdoor, breaking things in the process.
Packaging... let's not get started on this. "pip PTSD" is a thing
Of course, and that's exactly what we do at the runtime in the modularized Sage library; either with try... import ... except or lazy_import or Feature tests. The runtime does not care about the distributions, it only cares about the available functionality.
But at build time, whenever Cython modules are involved, things are more complicated!
but activate the "sagemath-pari" part only if pari is available, etc. IOW, keep sage the library as a whole, but make it more "dynamic" on what software it works with.
No, this is incompatible with Python packaging. Functionality is keyed to distribution names.
Why? Python itself is dynamic, and there are tons of python packages with functionality that is disabled or enabled on the presence of another python package.
As a quick example: mpmath optional dependency on gmpy2. In fact, mpmath backend defaults to python numbers. If a good enough gmpy2 is available (and MPMATH_NOMGPY is not set in the environment) then it will use gmpy2 as the backend (which is faster).
Have a look into .../site-packages/mpmath/libmp/backend.py.