Adapt length check in sage.databases.conway in preparation for database update
#37949
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
|
Might be good to add a comment to the doctest showing why its output can be two different things. Otherwise LGTM. |
Amend: Added comment explaining the relaxed length test
orlitzky
reviewed
May 7, 2024
| TESTS: | ||
|
|
||
| The database currently contains `35357` polynomials, but due to | ||
| :issue:`35357` it will be extended by Conway polynomials of |
There was a problem hiding this comment.
I was certainly expecting one of these 35357s to be a typo, but nope!
There was a problem hiding this comment.
I was also very surprised about the coincidence when I wrote it!
orlitzky
approved these changes
May 7, 2024
|
Thanks. I checked with the new |
|
Documentation preview for this PR (built with commit 2bc3197; changes) is ready! 🎉 |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 11, 2024
…eparation for database update
Motivated by sagemath#35357, the database of Conway polynomials will be enlarged
to ensure compatibility in the generation of finite fields (see
sagemath/conway-polynomials#5 for details). To
prepare for this update, we slightly relax the length check in
`sage.databases.conway` to be compatible with both the old and the new
version of the database.
URL: sagemath#37949
Reported by: Sebastian A. Spindler
Reviewer(s): Michael Orlitzky, Sebastian A. Spindler
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
May 12, 2024
…eparation for database update
Motivated by sagemath#35357, the database of Conway polynomials will be enlarged
to ensure compatibility in the generation of finite fields (see
sagemath/conway-polynomials#5 for details). To
prepare for this update, we slightly relax the length check in
`sage.databases.conway` to be compatible with both the old and the new
version of the database.
URL: sagemath#37949
Reported by: Sebastian A. Spindler
Reviewer(s): Michael Orlitzky, Sebastian A. Spindler
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.
Motivated by #35357, the database of Conway polynomials will be enlarged to ensure compatibility in the generation of finite fields (see sagemath/conway-polynomials#5 for details). To prepare for this update, we slightly relax the length check in
sage.databases.conwayto be compatible with both the old and the new version of the database.