Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

construct order ideals by default for number-field orders #38671

Merged

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Sep 18, 2024

This completes the change begun in #34806 and #35762: When constructing ideals of an order, return an ideal of that order, rather than a fractional ideal of the ambient number field.

Copy link

github-actions bot commented Sep 18, 2024

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

@yyyyx4 yyyyx4 force-pushed the public/construct_order_ideals_by_default branch from 8f51bee to fe7ae08 Compare September 18, 2024 14:17
@yyyyx4 yyyyx4 requested a review from pjbruin September 19, 2024 20:59
Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

LGTM.

Finally checking ci.

@yyyyx4
Copy link
Member Author

yyyyx4 commented Sep 25, 2024

Thank you!

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 27, 2024
…orders

    
This completes the change begun in sagemath#34806 and sagemath#35762: When constructing
ideals of an order, return an ideal of that order, rather than a
fractional ideal of the ambient number field.
    
URL: sagemath#38671
Reported by: Lorenz Panny
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit 5f1105c into sagemath:develop Sep 29, 2024
20 checks passed
@yyyyx4 yyyyx4 deleted the public/construct_order_ideals_by_default branch September 29, 2024 11:02
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.

3 participants