Skip to content

Conversation

S17A05
Copy link
Member

@S17A05 S17A05 commented May 6, 2024

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.conway to be compatible with both the old and the new version of the database.

@tscrim
Copy link
Collaborator

tscrim commented May 7, 2024

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
TESTS:

The database currently contains `35357` polynomials, but due to
:issue:`35357` it will be extended by Conway polynomials of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was certainly expecting one of these 35357s to be a typo, but nope!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also very surprised about the coincidence when I wrote it!

@orlitzky
Copy link
Contributor

orlitzky commented May 7, 2024

Thanks. I checked with the new conway-polynomials-0.10 and I get the same count.

Copy link

github-actions bot commented May 8, 2024

Documentation preview for this PR (built with commit 2bc3197; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

vbraun pushed a commit to vbraun/sage that referenced this pull request May 11, 2024
sagemathgh-37949: Adapt length check in `sage.databases.conway` in preparation 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
sagemathgh-37949: Adapt length check in `sage.databases.conway` in preparation 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 vbraun merged commit c9f5b45 into sagemath:develop May 12, 2024
@S17A05 S17A05 deleted the conway_length branch May 13, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants