src/sage/tests/gap_packages.py: remove #37076
Merged
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
…on debian-sid-minimal)
|
On top of the other PR, just removes the whole problematic file. @tobiasdiez I also dropped the "known failure" entry from your CI thingy. |
On OpenBSD this fails, they need something >0
antonio-rojas
approved these changes
Jan 21, 2024
…core' (for 64 GB)
…lity_platform_table.rst
added 15 commits
January 24, 2024 01:00
Not all of the added failures are conda-specific, but the 6 instances of CI Conda amplify the error probability (sagemath#36694 (comment)) We also add info (ideally a link to the GitHub Issue where the failure is reported) to the failures and arrange for it to be printed by the doctester as part of the `[failed in baseline]` message. Marked as blocker so it takes effect in CI runs -- to reduce the noise from failures of the Conda CI that PR authors and reviewers find distracting. <!-- ^^^^^ 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. - [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. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - Depends on sagemath#36936 (which refactors the `[failed in baseline]` printing) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36937 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
<!-- ^^^^^ 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 --> Furthering my hidden agenda of bringing portability tools to the masses. - We add `.devcontainer/portability-*/devcontainer.json` files for all platforms tested in our CI. This makes them selectable in GitHub Codespaces. - The files are generated by the new command `tox -e update_docker_platforms`, which also updates the workflow file `.github/workflows/docker.yml`. - The command also [adds a table to the portability testing section in the developer's guide](https://deploy-preview-36954-- sagemath.netlify.app/html/en/developer/portability_testing#using-our- pre-built-docker-images-published-on-ghcr-io) with links to GitHub Packages and friendly "Run in GitHub Codespaces" buttons. - Because 'tis the season of Spielzeugneid, [we document how to run the portability testing in GitHub Codespaces via Docker-in- Docker](https://deploy-preview-36954-- sagemath.netlify.app/html/en/developer/portability_testing#testing-sage- on-a-different-platform-using-docker), useful for those who cannot run Docker on their own computers. This uses a new devcontainer config "tox- docker-in-docker". - We make various other updates to the portability testing section in the developer's guide. <!-- 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. - [x] 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 <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - Depends on sagemath#37030 (split out from here) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36954 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez
<!-- ^^^^^ 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 --> https://github.com/numpy/numpy/releases/tag/v1.26.3 <!-- 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. --> - [ ] 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 <!-- 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#37000 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
<!-- ^^^^^ 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 --> https://github.com/OpenMathLib/OpenBLAS/releases/tag/v0.3.26 <!-- 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. --> - [ ] 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 <!-- 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#37005 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
using `Parent` for the Universal Cyclotomic Field
also adding some typing annotations in the modified file
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
URL: sagemath#37091
Reported by: Frédéric Chapoton
Reviewer(s):
Added the more natural notation P.x() and P.y() to access respectively the x and y coordinates of an EllipticCurvePoint. The behaviour mimics P.xy() and aims to replace P.xy()[0] and P.xy()[1]. Multiple instances of P.xy()[0] and P.xy()[1] are also replaced in related modules. #sd123 Done with @gioella URL: sagemath#37092 Reported by: Riccardo Invernizzi Reviewer(s): Lorenz Panny, Riccardo Invernizzi
E.g. on OpenBSD this fails, they need something >0
URL: sagemath#37104
Reported by: Dima Pasechnik
Reviewer(s):
…asering_coercion
In `sage.categories.unital_algebras.UnitalAlgebras`, remove the check
for the attribute `_no_generic_basering_coercion`, which was deprecated
in sagemath#19225. This means that this attribute is now completely ignored by
the coercion framework.
URL: sagemath#37115
Reported by: Peter Bruin
Reviewer(s): Marc Mezzarobba
Drop usage of some numpy function aliases which are no longer provided
in 1.12
URL: sagemath#37123
Reported by: Antonio Rojas
Reviewer(s): François Bissey, Gonzalo Tornaría
… divisors or (possibly irrational) points
In this patch we implement Algorithms 3 and 4 from [this
paper](https://ia.cr/2023/106). Evidently, the necessity for such an
algorithm arises when converting from endomorphism-ring ideals to
isogenies.
One immediate application is that `isogenies_prime_degree_general()` can
be simplified, even with a small speedup:
```sage
sage: K.<i> = QuadraticField(-1)
sage: E = EllipticCurve(K,[0,0,0,1,0])
sage: %timeit E.isogenies_prime_degree(37)
````
Before: `2.39 s ± 3.31 ms per loop (mean ± std. dev. of 7 runs, 1 loop
each)`
After: `2.29 s ± 5.05 ms per loop (mean ± std. dev. of 7 runs, 1 loop
each)`
#sd123
URL: sagemath#37125
Reported by: Lorenz Panny
Reviewer(s): John Cremona
<!-- ^^^^^ 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 --> as seen in ```sage sage: P2.<x,y,z> = ProjectiveSpace(QQ, 2) sage: S = P2.coordinate_ring() sage: I = S.ideal(0) sage: C = I.graded_free_resolution(); C S(0) <-- 0 sage: C[1] Ambient free module of rank 0 over the integral domain Multivariate Polynomial Ring in x, y, z over Rational Field sage: C[0] ------------------------------------------------------------------------ --- IndexError Traceback (most recent call last) Cell In[5], line 1 ----> 1 C[Integer(0)] File ~/GitHub/sage/src/sage/homology/free_resolution.py:426, in FiniteFreeResolution.__getitem__(self, i) 424 F = FreeModule(self._base_ring, 0) 425 elif i == self._length: --> 426 F = FreeModule(self._base_ring, self._maps[i - 1].ncols()) 427 else: 428 F = FreeModule(self._base_ring, self._maps[i].nrows()) IndexError: list index out of range sage: C.differential(1) ------------------------------------------------------------------------ --- IndexError Traceback (most recent call last) Cell In[6], line 1 ----> 1 C.differential(Integer(1)) File ~/GitHub/sage/src/sage/homology/free_resolution.py:489, in FiniteFreeResolution.differential(self, i) 487 elif i == self._length + 1: 488 s = FreeModule(self._base_ring, 0) --> 489 t = FreeModule(self._base_ring, self._maps[i - 2].ncols()) 490 m = s.hom(0, t) 491 elif i > self._length + 1: IndexError: list index out of range ``` <!-- 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. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [x] 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#37132 Reported by: Kwankyu Lee Reviewer(s): Travis Scrimshaw
slight modernisation of the modified file
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
URL: sagemath#37134
Reported by: Frédéric Chapoton
Reviewer(s): Kwankyu Lee
This is fixing a misbehaviour of empty tables, in their new unicode
clothes.
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
URL: sagemath#37147
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
This is just fixing some suggestions of ruff and pycodestyle in the
logic folder.
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
URL: sagemath#37154
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
Follow-up after: - sagemath#36670 This is working well, for example see https://github.com/mkoeppe/sage/ac tions/runs/6874334523/job/18695809308#step:11:5345, making it possible to obtain the image of the hanging build - https://github.com/mkoeppe/sa ge/actions/runs/6874334523/job/18695809308#step:15:25 However, `pkill` is not present on many of our `-minimal` configurations (which test our documented minimal build prerequisites). For example `debian-sid-minimal`, as seen in https://github.com/mkoeppe/sage/actions /runs/6874334523/job/18695811066#step:11:2262, rendering the mechanism ineffective there. (Neither are `ps` and `killall`.) Examples: - `docker run -it ghcr.io/sagemath/sage/sage-debian-bookworm-minimal- with-system-packages:dev bash -c "command -v pkill" ` - `docker run -it ghcr.io/sagemath/sage/sage-fedora-38-minimal-with- system-packages:dev bash -c "command -v pkill"` - whereas `docker run -it ghcr.io/sagemath/sage/sage-fedora-38-standard- with-system-packages:dev bash -c "command -v pkill" ` --> `/usr/bin/pkill`. We replace `pkill` by more primitive means. Test run (with timeout set to ~11 minutes for testing purposes): https:/ /github.com/mkoeppe/sage/actions/runs/6883232154/job/18723359505#step:15 :25 <!-- ^^^^^ 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. - [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. ### ⌛ 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#36726 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Nathan Dunfield, William Stein
orlitzky
force-pushed
the
dont-test-all-installed-gap-packages
branch
from
135a98e
to
4560e05
Compare
|
rebased onto develop |
These tests for the installed GAP packages are problematic in a world where Sage can use GAP from the system. Particularly, loading _all_ installed GAP packages is not a safe operation when there might be an enormous number of them. Moreover, with the system GAP, it's not really Sage's reponsibility to ensure that the installed packages load. It _would_ be nice to know that any extra packages from the gap_packages SPKG are installed correctly; however, that type of test would be more appropriate in that SPKG's spkg-check phase.
orlitzky
force-pushed
the
dont-test-all-installed-gap-packages
branch
from
4560e05
to
8d6bdbf
Compare
|
Oh, it was conflicting with your own develop branch. I rebased it on top of that:
|
|
Documentation preview for this PR (built with commit 8d6bdbf; changes) is ready! 🎉 |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jan 27, 2024
These tests for the installed GAP packages are problematic in a world where Sage can use GAP from the system. Particularly, loading _all_ installed GAP packages is not a safe operation when there might be an enormous number of them. Moreover, with the system GAP, it's not really Sage's reponsibility to ensure that the installed packages load. It _would_ be nice to know that any extra packages from the gap_packages SPKG are installed correctly; however, that type of test would be more appropriate in that SPKG's spkg-check phase. Dependencies: - sagemath#37049 URL: sagemath#37076 Reported by: Michael Orlitzky Reviewer(s): Antonio Rojas
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jan 29, 2024
These tests for the installed GAP packages are problematic in a world where Sage can use GAP from the system. Particularly, loading _all_ installed GAP packages is not a safe operation when there might be an enormous number of them. Moreover, with the system GAP, it's not really Sage's reponsibility to ensure that the installed packages load. It _would_ be nice to know that any extra packages from the gap_packages SPKG are installed correctly; however, that type of test would be more appropriate in that SPKG's spkg-check phase. Dependencies: - sagemath#37049 URL: sagemath#37076 Reported by: Michael Orlitzky Reviewer(s): Antonio Rojas
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jan 30, 2024
These tests for the installed GAP packages are problematic in a world where Sage can use GAP from the system. Particularly, loading _all_ installed GAP packages is not a safe operation when there might be an enormous number of them. Moreover, with the system GAP, it's not really Sage's reponsibility to ensure that the installed packages load. It _would_ be nice to know that any extra packages from the gap_packages SPKG are installed correctly; however, that type of test would be more appropriate in that SPKG's spkg-check phase. Dependencies: - sagemath#37049 URL: sagemath#37076 Reported by: Michael Orlitzky Reviewer(s): Antonio Rojas
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.
These tests for the installed GAP packages are problematic in a world where Sage can use GAP from the system. Particularly, loading all installed GAP packages is not a safe operation when there might be an enormous number of them.
Moreover, with the system GAP, it's not really Sage's reponsibility to ensure that the installed packages load. It would be nice to know that any extra packages from the gap_packages SPKG are installed correctly; however, that type of test would be more appropriate in that SPKG's spkg-check phase.
Dependencies: