Skip to content

Conversation

saraedum
Copy link
Member

@saraedum saraedum commented Apr 29, 2024

This PR recreates #36676 by @mkoeppe after it was reverted, see #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.py
  • src/sage/misc/all__sagemath_objects.py
  • src/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).

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.

📝 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.

⌛ Dependencies

Matthias Koeppe and others added 24 commits April 14, 2024 21:56
(cd src && for d in $(find sage -name __pycache__ -prune -o -type d -print); do sed -i.bak "/from *[.].*import/s/from /from $(echo $d | sed 's,/,.,g')/;s/[.] import / import /;" $d/all*.py; done)
…o if ! grep -q 'del lazy_import' $a; then echo 'del lazy_import' >> $a; fi; done
…o if ! grep -q 'del install_doc' $a; then echo 'del install_doc' >> $a; fi; done
The chain of immediate ancestors of this commit was created by
cherry-picking the relevant commits that comprise sagemath#36676. Replaying all
these commits (and the necessary conflict resolution) still leaves a
non-empty diff when comparing 10.4.beta2 to the previous sagemath#36676 and
10.4.beta3 to that cherry-picked branch. This commit makes sure that the
diff is trivial. The changes introduced here were likely introduced in
conflict resolution when merging develop into sagemath#36676. I did not replay
these conflict resolutions, so I do this here manually.
@saraedum
Copy link
Member Author

Last count of votes from #36676:

👍 @mkoeppe @kwankyu @culler @jhpalmieri @kiwifb @NathanDunfield @GMS103 @roed314
👎 @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.)

@mkoeppe
Copy link
Contributor

mkoeppe commented May 2, 2024

separate sagemath-foobar submodules of sagelib have very little value by themselves

@dimpase The deliberate design (https://groups.google.com/g/sage-devel/c/kiB32zP3xD4) is that one combines several distributions at runtime, to avoid a combinatorial explosion. I explained this clearly again a week ago in the sage-devel thread https://groups.google.com/g/sage-devel/c/mqgtkLr2gXY/m/SYQWB27hAAAJ -- after you declared the distributions "useless" the first time (https://groups.google.com/g/sage-devel/c/mqgtkLr2gXY/m/_iD6dkbdAAAJ). Oh, and in https://groups.google.com/g/sage-devel/c/mqgtkLr2gXY/m/aGsSdQ7GBAAJ you emphasized the importance of making binary wheels (until I pointed out that I'm making them), only to turn around in https://groups.google.com/g/sage-devel/c/mqgtkLr2gXY/m/5x8gjw33AAAJ to say that you "don't see the point of distributing such pieces, as Python wheels, as something useful on their own."
"On their own" - that's a strawman. And nothing is gained by repeating it once more here in this PR.

@dimpase

This comment was marked as abuse.

@kwankyu
Copy link
Collaborator

kwankyu commented May 2, 2024

In a way, RJF, a UC Berkeley professor of CS, was not incorrect in his scepticism.

Sage with "a huge collection of bugs" had not even existed by now if @williamstein blindly believed his ("rjf") skepticism.

@dimpase
Copy link
Member

dimpase commented May 2, 2024

In a way, RJF, a UC Berkeley professor of CS, was not incorrect in his scepticism.

Sage with "a huge collection of bugs" had not even existed by now if @williamstein blindly believed his ("rjf") skepticism.

please recall that Sage hasn't achieved its original mission. In that sense RJF has been 100% correct.

It's a good indication of the state of the project that we can't even agree on a new mission statement :-)

@kwankyu
Copy link
Collaborator

kwankyu commented May 2, 2024

... we can't even agree on a new mission statement

By the way, in #36827, people spent much time and energy proposing new statements. Their time was only wasted because you didn't even try to come up with some candidates for which people can vote for on sage-devel.

@dimpase
Copy link
Member

dimpase commented May 2, 2024

... we can't even agree on a new mission statement

By the way, in #36827, people spent much time and energy proposing new statements. Their time was only wasted because you didn't even try to come up with some candidates for which people can vote for on sage-devel.

I saw no way to reconcile the opinions there.

@jhpalmieri

This comment was marked as off-topic.

SageMath version 10.4.beta5, Release Date: 2024-05-02
vbraun pushed a commit to vbraun/sage that referenced this pull request May 15, 2024
sagemathgh-37993: `sage.calculus.expr`: Split out from `sage.calculus.all`
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Cherry-picked from sagemath#36676/sagemath#37900.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37993
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request May 18, 2024
sagemathgh-37993: `sage.calculus.expr`: Split out from `sage.calculus.all`
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Cherry-picked from sagemath#36676/sagemath#37900.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37993
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request May 18, 2024
sagemathgh-37993: `sage.calculus.expr`: Split out from `sage.calculus.all`
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Cherry-picked from sagemath#36676/sagemath#37900.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37993
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request May 24, 2024
sagemathgh-37993: `sage.calculus.expr`: Split out from `sage.calculus.all`
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Cherry-picked from sagemath#36676/sagemath#37900.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37993
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request May 24, 2024
sagemathgh-38030: Relative to absolute imports, `del lazy_import` in `.all` files
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Split out from sagemath#37900

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38030
Reported by: Matthias Köppe
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request May 29, 2024
sagemathgh-38086: `all*.py` files: Use 'del lazy_import', 'del install_doc'
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Cherry-picked from sagemath#37900

Includes style fixes from there as well.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38086
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request May 29, 2024
sagemathgh-38095: `src/sage/algebras/steenrod/all.py`: Use lazy_import, remove deprecated global import
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- The change to use `lazy_import` was cherry-picked from sagemath#37900
- The global import was deprecated in sagemath#32647 (2021)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38095
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request May 31, 2024
sagemathgh-38086: `all*.py` files: Use 'del lazy_import', 'del install_doc'
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Cherry-picked from sagemath#37900

Includes style fixes from there as well.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38086
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request May 31, 2024
sagemathgh-38095: `src/sage/algebras/steenrod/all.py`: Use lazy_import, remove deprecated global import
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- The change to use `lazy_import` was cherry-picked from sagemath#37900
- The global import was deprecated in sagemath#32647 (2021)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38095
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
sagemathgh-38096: `sage.combinat`, `sage.games`: Use more `lazy_import` in all.py files
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This helps with the modularization, but is also desirable for reducing
the start-up time of Sage.
- Cherry-picked from sagemath#37900

Related: sagemath#35520

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38096
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
sagemathgh-38096: `sage.combinat`, `sage.games`: Use more `lazy_import` in all.py files
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This helps with the modularization, but is also desirable for reducing
the start-up time of Sage.
- Cherry-picked from sagemath#37900

Related: sagemath#35520

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38096
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 17, 2024
sagemathgh-38096: `sage.combinat`, `sage.games`: Use more `lazy_import` in all.py files
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This helps with the modularization, but is also desirable for reducing
the start-up time of Sage.
- Cherry-picked from sagemath#37900

Related: sagemath#35520

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38096
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ s: needs review v: large
Projects
None yet
Development

Successfully merging this pull request may close these issues.