Demote brial (= polybori) from standard to optional, add distribution sagemath-brial, enlarge sagemath-objects, sagemath-categories #36380
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
docs don't build. I think one can't build boolean polynomials' docs without brial installed, so this has to be moved out: --- a/src/doc/en/reference/polynomial_rings/index.rst
+++ b/src/doc/en/reference/polynomial_rings/index.rst
@@ -62,12 +62,4 @@ Infinite Polynomial Rings
sage/rings/polynomial/symmetric_ideal
sage/rings/polynomial/symmetric_reduction
-Boolean Polynomials
--------------------
-
-.. toctree::
- :maxdepth: 1
-
- sage/rings/polynomial/pbori/pbori
-
.. include:: ../footer.txt |
|
Right. We will need to find a solution for this more general problem of building the manual with optional parts... |
|
also: |
|
there are several unconditional imports of BooleanPolynomials left, e.g. in Should it go via an abstract class, etc, to achieve modularity ? |
I think I have already done all necessary changes of this kind in #35095. I'll cherry-pick them to this branch later today. |
|
Actually I created the ABC Edit: Done already in f5c6196 |
mkoeppe
force-pushed
the
brial_experimental
branch
from
ff00c0d
to
4fc966f
Compare
mkoeppe
force-pushed
the
brial_experimental
branch
from
66c5c6f
to
cc9077e
Compare
kwankyu
reviewed
Oct 9, 2023
.gitignore
Outdated
| @@ -215,6 +217,7 @@ build/bin/sage-build-env-config | |||
| /pkgs/sagemath-categories/requirements.txt | |||
| /pkgs/sagemath-environment/requirements.txt | |||
| /pkgs/sagemath-repl/requirements.txt | |||
| /pkgs/sagemath-brial/requirements*.txt | |||
| /pkgs/sagemath-categories/MANIFEST.in | |||
|
|
|||
There was a problem hiding this comment.
OK for now. But I think this block of lines should be sorted out by distribution names...
kwankyu
reviewed
Oct 9, 2023
kwankyu
reviewed
Oct 9, 2023
kwankyu
reviewed
Oct 9, 2023
mkoeppe
force-pushed
the
brial_experimental
branch
from
bd8dc76
to
bba5e50
Compare
kwankyu
reviewed
Oct 14, 2023
|
I get $ ./sage -sh -c '(cd pkgs/sagemath-brial && tox -v -v -v -e sagepython)'
...
cypari2/closure.pyx:139:4: 'sig_on' is not a constant, variable or function identifier
Error compiling Cython file:
------------------------------------------------------------
...
cdef int _pari_init_closure() except -1:
sig_on()
global ep_call_python
ep_call_python = install(<void*>call_python, "call_python", 'DGDGDGDGDGD5,U,U')
sig_off()
^
------------------------------------------------------------
cypari2/closure.pyx:142:4: 'sig_off' is not a constant, variable or function identifier
Error compiling Cython file:
------------------------------------------------------------
...
# Only 5 arguments are supported for now...
if nargs > 5:
nargs = 5
# Fill in default arguments of PARI function
sig_on()
^
------------------------------------------------------------
cypari2/closure.pyx:230:4: 'sig_on' is not a constant, variable or function identifier
Traceback (most recent call last):
...
File "/private/var/folders/td/fw1q9ljs311ggyph77rs53_40000gn/T/pip-install-h6yasewl/cypari2_0d737fa76b3f479ea5e1b6ab73dffe77/.eggs/Cython-3.0.4-py3.11.egg/Cython/Build/Dependencies.py", line 1321, in cythonize_one
raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: cypari2/closure.pyx
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: legacy-install-failure
× Encountered error while trying to install package.
╰─> cypari2
note: This is an issue with the package mentioned above, not pip.
hint: See above for output from the failure.
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error
× pip subprocess to install build dependencies did not run successfully.
│ exit code: 1
╰─> See above for output.
note: This error originates from a subprocess, and is likely not a problem with pip.
sagepython: 67309 C exit 1 (66.94 seconds) /Users/kwankyu/GitHub/sage-dev/pkgs/sagemath-brial> python -I -m pip install -r requirements.txt pid=54141 [tox/execute/api.py:275]
sagepython: FAIL code 1 (66.95 seconds)
evaluation failed :( (67.15 seconds) |
|
This work? There's no doc for it... |
tobiasdiez
reviewed
Oct 19, 2023
mkoeppe
force-pushed
the
brial_experimental
branch
from
8a2be7d
to
8b04845
Compare
|
Documentation preview for this PR (built with commit 876fa67; changes) is ready! 🎉 |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 24, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> Documentation is conditionalized by using Sphinx tags. Split out from (and preparation for): - sagemath#36380 Part of: - sagemath#29705 <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [ ] 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. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36495 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 24, 2023
… pattern
As requested in
sagemath#36380 (comment) and
cherry-picked from there.
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->
- [x] The title is concise, informative, and self-explanatory.
- [ ] 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.
### ⌛ Dependencies
<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
URL: sagemath#36511
Reported by: Matthias Köppe
Reviewer(s): Tobias Diez
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 26, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> Documentation is conditionalized by using Sphinx tags. Split out from (and preparation for): - sagemath#36380 Part of: - sagemath#29705 <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [ ] 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. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36495 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 26, 2023
… pattern
As requested in
sagemath#36380 (comment) and
cherry-picked from there.
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->
- [x] The title is concise, informative, and self-explanatory.
- [ ] 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.
### ⌛ Dependencies
<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
URL: sagemath#36511
Reported by: Matthias Köppe
Reviewer(s): Tobias Diez
|
"Disputed" was set 3 weeks ago.
|
|
Needs rebasing |
No, it's up to date |
mantepse
reviewed
Sep 27, 2024
There was a problem hiding this comment.
Could you please explain why this file is needed and where the name of the file comes from? What does taylor have to do with categories?
There was a problem hiding this comment.
sagemath-categories per definition in its README provides "Sage categories, basic rings, polynomials, functions".
There was a problem hiding this comment.
In general, the all__*.py define the top-level interactive environment that is used for running the doctests.
There was a problem hiding this comment.
Concretely, the imports from sage.calculus.functional are global functions that dispatch to like-named methods. They are usable with polynomials, for example, which are provided by the enlarged sagemath-categories.
There was a problem hiding this comment.
Yes, but expand, simplify and taylor are not in "Sage categories, basic rings, polynomials, functions".
There was a problem hiding this comment.
factoris not provided bysagemath_categories, is it?
Well, the class Factorization is provided already by sagemath-objects (dependency of sagemath-categories).
There was a problem hiding this comment.
In what way are
simplify,expandandtaylorusable with polynomials?
simplify and expand are no-ops with polynomials. taylor works with polynomials as it does with symbolic expressions.
Note that the current implementation of some of these methods goes through SR. That's a typical modularization obstacle that should be addressed eventually in a follow-up PR.
There was a problem hiding this comment.
OK, but what do these function have to do with sagemath_categories? I thought that this should be a rather minimal subset of sage, and simplify is something that I would certainly not expect to use on polynomials (as: elements of an exact polynomial ring with zero test).
There was a problem hiding this comment.
Consider that there may be a wide range of expectations among Sage users. I'd say it's unwise to generalize from your specific expectations.
There was a problem hiding this comment.
"Minimality" is not a goal for sagemath-categories, except regarding its dependencies.
|
There are many optional tags missing in doctests. With brial uninstalled (so |
|
@antonio-rojas Thanks for testing! Note that files such as |
|
(I'll look into the failures in the other files.) |
I understard. So far on Arch I've been shipping sagemath-standard together with all optional distributions in a single distro package. Optional extensions are then enabled by installing the respective shared libraries. Since sage features test for modules being importable (not just present) this works nicely as long as doctest are properly tagged. |
|
OK, I have added the doctest tags for the files in |
added 5 commits
September 28, 2024 16:18
mkoeppe
force-pushed
the
brial_experimental
branch
from
74e4d1f
to
0a400bf
Compare
|
@antonio-rojas Tests should be better now |
Thanks, looks good now |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 5, 2024
…n features
<!-- ^ 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#36380.
### 📝 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#38731
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 5, 2024
<!-- ^ 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#36380. ### 📝 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#38732 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 5, 2024
<!-- ^ 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#36380. ### 📝 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#38733 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 5, 2024
<!-- ^ 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#35095 and sagemath#36380. ### 📝 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#38751 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
as proposed in https://groups.google.com/g/sage-devel/c/ewHP20bmskQ, because (among other things):
It has been dropped from Debian testing (
debian-trixie) andubuntu-noble, where it seems to block SageMath upgrades (Sage is stuck at 9.5 in Debian/Ubuntu). See https://repology.org/project/brial/versionssage.rings.polynomial.pboriThe new distribution builds on top of enlarged distributions sagemath-objects, sagemath-categories. This is a major update of these distributions, which makes them much more useful.
Also includes style changes to
all*.pyfiles cherry-picked from #37900.📝 Checklist
⌛ Dependencies
Depends onpkgs/sagemath-{objects,categories}: Enlarge #36566 (split out from here)# sage_setup: distributiondirectives for the files of existing distributions sagemath-{categories,...,tdlib} #38088 (merged here)sage.monoids: Modularization fixes,# needs#38143 (merged here)sage.schemes: Modularization fixes (imports) #38142 (merged here)sage.interfaces: Morelazy_import#38180 (split out from here)