sage
sage copied to clipboard
Restructure sage.*.all for modularization, replace relative by absolute imports
This PR recreates https://github.com/sagemath/sage/pull/36676 by @mkoeppe after it was reverted, see https://github.com/sagemath/sage/pull/37796.
We restructure the all.py files for modularization.
Guided by the technical constraints outlined in https://groups.google.com/g/sage-devel/c/ozh-7ZZ848s, #35095 defines distribution packages (pip-installable packages) sagemath-{brial, combinat, eclib, flint, gap, giac, glpk, graphs, groups, homfly, lcalc, libbraiding, libecm, linbox, modules, mpmath, ntl, pari, plot, polyhedra, schemes, singular, standard-no-symbolics, symbolics}.
When a namespace package such as sage.misc is filled by modules from several distribution packages, we create modules named:
src/sage/misc/all__sagemath_environment.pysrc/sage/misc/all__sagemath_objects.pysrc/sage/misc/all__sagemath_repl.py
Import statements are moved from src/sage/misc/all.py to these files as appropriate, and src/sage/misc/all.py imports * from there.
Also some imports are replaced by lazy imports.
The new files provide the top level namespaces for the modularized distribution packages, thus enabling modularized testing.
This design was introduced in #29865 (merged in Jan 2022, early in the Sage 9.6 development cycle).
- Copied from #35095.
- Part of #29705
Moreover, applied a one-line command to replace relative by absolute imports, thus complementing #36666, which does not touch .all* files.
The changes to other files in sage.modules etc. come from the PRs #36597, #36572, #36588, #36589 merged here and do not need review.
:memo: Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
:hourglass: Dependencies
- Depends on #36597 (merged here to resolve merge conflict)
- Depends on https://github.com/sagemath/sage/pull/36572 (merged here to resolve merge conflict)
- Depends on https://github.com/sagemath/sage/pull/36588 (merged here to resolve merge conflict)
- Depends on https://github.com/sagemath/sage/pull/36589 (merged here to resolve merge conflict)
- Depends on #36449 (merged here to resolve merge conflict)
- Depends on #36390 (merged here to resolve merge conflict)
- Depends on #38030 (split out from here)
Last count of votes from #36676:
:+1: @mkoeppe @kwankyu @culler @jhpalmieri @kiwifb @NathanDunfield @GMS103 @roed314 :-1: @tobiasdiez @dimpase @tornaria @mantepse @orlitzky
(Technically, I am the "author" of this PR but I just recreated it. I did not vote on this PR.)
Documentation preview for this PR (built with commit 8865085e6c26e92b3038fb42d65535541a86315c; changes) is ready! :tada: This preview will update shortly after each push to this PR.
I think I did not want to vote on this PR, but rather on what is now #37901. I am sorry that I am confused. To make it precise, I am only against splitting the mathematics into subfields without a very clear argument why this is the best way to go.
Please excuse any confusion and troubles I am causing on top of everything. I don't really understand in-depth what the present PR is about. If this PR implies the other (I don't see this in the code, but it is mentioned in the description of the PR), I stick to my vote here.
PS: If the present PR is indeed not implying #37901, the description should be updated accordingly, I think.
In light of @mantepse withdrawing their vote, the count is now 8-4 in favor of this PR. Setting positive review.
👍 @mkoeppe @kwankyu @culler @jhpalmieri @kiwifb @NathanDunfield @GMS103 @roed314 👎 @tobiasdiez @dimpase @tornaria @orlitzky
I think I did not want to vote on this PR, but rather on what is now #37901. I am sorry that I am confused. To make it precise, I am only against splitting the mathematics into subfields without a very clear argument why this is the best way to go.
This PR does propose to split the mathematics part of sage, see eg https://github.com/sagemath/sage/pull/37900/files#diff-b804fdce0160caae377fc0f952c4567fbfd698a4817e07f43c3c1911cff4c6a2. In this PR, the splitting is on the level of the all.py files which is direct preparation for the splitting as "modules" as currently contained in the monster PR https://github.com/sagemath/sage/pull/35095.
Could anyone voting with +1 here please explain me why they like the additional all_xyz files over some form of splitting of the existing all.py file in the form:
if sagemath_categories is installed:
import everything specific to that distribution
if sagemath_other_module is installed:
import ...
Some of the new all files are even empty, so clearly not needed for the goal of this PR ("The new files provide the top level namespaces for the modularized distribution packages, thus enabling modularized testing.")
Could anyone voting with +1 here please explain me why they like the additional
all_xyzfiles over some form of splitting of the existingall.pyfile in the form:
I'll just note that this demand by Tobias that my existing, tested, documented design be given equal consideration to an untested, vague alternative proposal is exactly what put this PR into "disputed status" in the first place. https://github.com/sagemath/sage/pull/36676#pullrequestreview-1726079717 (November 2023); https://github.com/sagemath/sage/pull/36676#issuecomment-1849216706
Could anyone voting with +1 here please explain me why they like the additional
all_xyzfiles over some form of splitting of the existingall.pyfile in the form:I'll just note that this demand by Tobias that my existing, tested, documented design be given equal consideration to an untested, vague alternative proposal is exactly what put this PR into "disputed status" in the first place. #36676 (review) (November 2023); #36676 (comment)
This is certainly one reason to support the current PR: it is an actual concrete proposal that accomplishes the task.
Since I only withdrew my vote on the basis that this PR does not split the mathematics library, I set back the label. The votes are, therefore, I think,
👍 @mkoeppe @kwankyu @culler @jhpalmieri @kiwifb @NathanDunfield @GMS103 @roed314 👎 @tobiasdiez @dimpase @tornaria @orlitzky @mantepse
PS: I must say that it makes me rather uncomfortable that, instead of clarifying the situation, the PR is set to "positive review". I do understand, however, that it's a bit hard to actually know whether what I object to materialises in this PR or only in the other - after all, I don't.
this existing, documented, etc etc thing is just too gross to be included in sage.
It's hard to imagine something more un-pythonic.
this existing, documented, etc etc thing is just too gross to be included in sage.
It's hard to imagine something more un-pythonic.
Such denunciations, apart from the fact that they have no basis whatsoever, have no place in the Sage community.
I do understand, however, that it's a bit hard to actually know whether what I object to materialises in this PR or only in the other - after all, I don't.
Generally voters are responsible themselves for the vote that they cast.
Generally voters are responsible themselves for the vote that they cast.
Matthias, I find this quite condescending.
Martin, why would you say so, would you elaborate?
This is stating a fact that is well known and undisputed, as if I wouldn't know it.
It is not stating the fact as an argument in a dispute, because it leaves out the most relevant context: I stated that I am withdrawing my vote based on a certain assumption, which is actually not satisfied.
This is stating a fact that is well known and undisputed, as if I wouldn't know it.
I'm sure you know it, but consider that the point may be that you weren't acting according to this knowledge. You decided to cast an unclear, "conditional" vote that put others in charge of interpreting it; and then as someone did interpret it, you criticized them, saying that that made you uncomfortable.
I find it hard to see how the condition I stated was unclear. And yes, I was aware of my abuse, I even stated that in my comment, and asked for forgiveness.
If the point of your comment was that I should not ask others with a better understanding of the PR for checking my condition, I would prefer if you state it so.
However, I think it would also be the responsibility of the author of the PR to clarify the fact whether the PR commences to split the math library or not. For example, you could have pointed out the two lines instead of Tobias doing it.
PS: the sentence containing the word "uncomfortable" was also qualified with a sentence that I understand what @NathanDunfield did.
this existing, documented, etc etc thing is just too gross to be included in sage.
It's hard to imagine something more un-pythonic.
Such denunciations, apart from the fact that they have no basis whatsoever, have no place in the Sage community.
you have turned Sage development into an all-out pub brawl. Enjoy.
... to clarify the fact whether the PR commences to split the math library or not.
No. This PR does a part of the modularization project which aims to "split the math library", but this PR did not start it.
It seems that you are abhorred by "splitting the math library", which is what the modularization project is doing for years by now. If you want to stop it, sage-devel is the place, not the individual PRs accomplishing it, for your campaign.
But there is nothing to be scary in "splitting the math library". Yes, math is a unity. But doing math or computing math is not. I use some part of sage, but never used larger part of the sage library. Nonetheless, I spend (and perhaps will spend) time to install the sage that includes all dependencies to support the whole math library. But the modularization project is for other people who want to install and use only a small part of the sage library within the python ecosystem.
... to clarify the fact whether the PR commences to split the math library or not.
No. This PR does a part of the modularization project which aims to "split the math library", but this PR did not start it.
OK, thank you for the clarification.
It seems that you are abhorred by "splitting the math library", which is what the modularization project is doing for years by now. If you want to stop it, sage-devel is the place, not the individual PRs accomplishing it, for your campaign.
I did post my concerns on sage-devel.
But there is nothing to be scary in "splitting the math library". Yes, math is a unity. But doing math or computing math is not. I use some part of sage, but never used larger part of the sage library. Nonetheless, I spend (and perhaps will spend) time to install the sage that includes all dependencies to support the whole math library. But the modularization project is for other people who want to install and use only a small part of the sage library within the python ecosystem.
Making dependencies optional is not what I am concerned about. I am exclusively, but very sceptical about splitting the math library into:
- sagemath-combinat: everything combinatorial except for graphs.
- sagemath-graphs: also provides posets, designs, abstract complexes, quivers, etc.
- sagemath-groups
- sagemath-modules: also has matrices, tensors, homology, coding theory, etc.
- sagemath-polyhedra: also provides fans, hyperplane arrangements, etc.
- sagemath-schemes
- sagemath-symbolics
The only real reason for this split that I was given is "discoverability". I think that this does not justify the costs of the split, in particular an increased barrier to use functionality from other parts of sage, or, alternatively, breaking promises implicitly made to consumers. The example easiest to understand for me are combinatorial species, which are currently in sagemath-combinat, and are of potential interest to consumers, see, for example http://combos.org. After this summer, if all goes well, and if I understand my mathematics well enough, even the present functionality will probably depend on gap, because this will allow things to be done more rigorously.
I have explained already why this maths split doesn't fly. E.g. without a good 2/3rds of the rest of sagelib, neither sagemath-graphs nor sagemath-combinat are really functional.
... I think that this does not justify the costs of the split, in particular an increased barrier to use functionality from other parts of sage, or, alternatively, breaking promises implicitly made to consumers.
There are no consumers yet, as there is no product yet. A researcher like you would be the biggest consumer of sagemath-combinat.
The example easiest to understand for me are combinatorial species, which are currently in sagemath-combinat, and are of potential interest to consumers, see, for example http://combos.org. After this summer, if all goes well, and if I understand my mathematics well enough, even the present functionality will probably depend on gap, because this will allow things to be done more rigorously.
The current (proposed) split of the listed distribution packages is not the end of the story. The story would go on with lots of discussions on how to split the sage library, or how to organize the distribution packages for the best satisfaction of the consumers.
Those discussions would be a fun part of the story of the modularization. But even to start the discussions, the primitive design (even imperfect) of the modularization should be in place into sage, first. This did not happen yet, because of the current disputes.
Hello,
It is really hard for me to understand the issues when tension arises, and I suspect that I am not the only one.
As a member of the Code of Conduct Committee, I (meaning that I am acting without necessarily checking with the entire committee) would like to give the advice to strive to adhere to the Code of Conduct when writing comments here, otherwise the Code of Conduct Committee may be summoned to take the necessary actions in accordance with the suggestions that was agreed upon by the community. Patience and being welcoming are important aspect to keep in mind for the current discussion.
I have explained already why this maths split doesn't fly. E.g. without a good 2/3rds of the rest of sagelib, neither sagemath-graphs nor sagemath-combinat are really functional.
@dimpase Will you also explain to us on the basis of what work you would come to such conclusions?
large chunks of code in e.g. sagemath-graphs need number theory, numerical optimisation, linear algebra, designs from sagemath-combinat to function.
it's pointless to say that "these are lazy-imported". It doesn't make the sagemath-graphs magically working. All these missing pieces need to be installed.
large chunks of code in e.g. sagemath-graphs need number theory, numerical optimisation, linear algebra, designs from sagemath-combinat to function.
Yes, but how "large" is "large"? Is it 5%, 30%, 50%, 70%, 95%? Can you say, and based on doing what work, Dima?
By the way, combinatorial designs are not in sagemath-combinat. They are in sagemath-graphs; it says so in the README: https://github.com/mkoeppe/sage/tree/t/32432/modularization_of_sagelib__break_out_a_separate_package_sagemath_polyhedra/pkgs/sagemath-graphs
it's pointless to say that "these are lazy-imported".
Neither is it pointless, nor is it the full mechanism. "lazy_import" is one of several techniques that make modules importable.
It doesn't make the sagemath-graphs magically working. All these missing pieces need to be installed.
No, they don't. Only those pieces that are needed by the parts of sagemath-graphs that you want to use need to be installed.
The example easiest to understand for me are combinatorial species, which are currently in sagemath-combinat
@mantepse Happy to explain it on this example. Yes, sage.combinat.species is shipped by sagemath-combinat.
If you do git grep 'needs sage' src/sage/combinat/species, you see that various doctests that illustrate its functionality are tagged with # needs sage.graphs, # needs sage.modules, # needs sage.groups, etc.
If you want to advertise full combinatorial species functionality including whatever you may need from GAP in the future, we would simply define an "extra" species so that people can say pip install sagemath-combinat[species] to mean the same as pip install sagemath-combinat sagemath-polyhedra sagemath-groups sagemath-graphs. That's all.
If it turns out in the future that neither you nor other developers are interested in making parts of the combinatorial species functionality available when not all of these dependencies are available, then we can simply mark all of the files with one big file-level # needs sage.graphs sage.modules sage.groups ..., and that's that. Zero cost.
Could anyone voting with +1 here please explain me why they like the additional
all_xyzfiles over some form of splitting of the existingall.pyfile in the form: [ ... ] Some of the new all files are even empty, so clearly not needed for the goal of this PR ("The new files provide the top level namespaces for the modularized distribution packages, thus enabling modularized testing.")
For me, it is not really a matter of liking the additional files better than something else. The additional files are serving a purpose (faciilitating testing of individual modules). And their existence does not offend or upset me. After all, every python package in the universe, whether in sage or not, contains a file named __init__.py. Moreover, many of those __init__.py files are empty. They are needed, even when they are empty.
If it turns out in the future that neither you nor other developers are interested in making parts of the combinatorial species functionality available when not all of these dependencies are available, then we can simply mark all of the files with one big file-level
# needs sage.graphs sage.modules sage.groups ..., and that's that. Zero cost.
I don't think that this has zero cost, because it is breaking a promise. If we advertise, for example, sagemath-combinat, we advertise it with the current functionality. However, if dependencies change, it may not be possible to use this functionality at all anymore in certain settings.
If it turns out in the future that neither you nor other developers are interested in making parts of the combinatorial species functionality available when not all of these dependencies are available, then we can simply mark all of the files with one big file-level
# needs sage.graphs sage.modules sage.groups ..., and that's that. Zero cost.I don't think that this has zero cost, because it is breaking a promise. If we advertise, for example,
sagemath-combinat, we advertise it with the current functionality. However, if dependencies change, it may not be possible to use this functionality at all anymore in certain settings.
Indeed. And users don't tend to read source code, unless pressed.
Besides, all that "this is better discoverability for the users", "no, it's actually not for for the users, it's just for developers" constant shifting of the goalposts we see as justification of the sagelib modularisation does not look good, it's just attempts to defend something dodgy.