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

add .basis_matrix() wrapper to quaternion orders #35767

Conversation

yyyyx4
Copy link
Member

@yyyyx4 yyyyx4 commented Jun 14, 2023

Computing the basis matrix of a quaternion order is a common operation, but it currently requires building it "by hand" from the .basis(). This straightforward patch adds a method O.basis_matrix() which computes matrix(QQ, map(list, O.basis())).

Eventually, both orders and ideals should become children of a common "quaternion lattice" class; this patch is a quick hack to reduce the pain until that has happened.

@yyyyx4 yyyyx4 marked this pull request as draft June 14, 2023 10:31
@yyyyx4 yyyyx4 marked this pull request as ready for review June 14, 2023 10:39
@yyyyx4 yyyyx4 force-pushed the public/add_basis_matrix_method_to_quaternion_orders branch from e7d68cf to 53b776f Compare August 21, 2023 20:52
@github-actions
Copy link

Documentation preview for this PR (built with commit 53b776f; changes) is ready! 🎉

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

LGTM.

@tscrim tscrim added this to the sage-10.2 milestone Sep 13, 2023
vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 14, 2023
    
Computing the basis matrix of a quaternion order is a common operation,
but it currently requires building it "by hand" from the `.basis()`.
This straightforward patch adds a method `O.basis_matrix()` which
computes `matrix(QQ, map(list, O.basis()))`.

Eventually, both orders and ideals should become children of a common
"quaternion lattice" class; this patch is a quick hack to reduce the
pain until that has happened.
    
URL: sagemath#35767
Reported by: Lorenz Panny
Reviewer(s): Travis Scrimshaw
@vbraun vbraun merged commit 3a3126f into sagemath:develop Sep 16, 2023
@yyyyx4 yyyyx4 deleted the public/add_basis_matrix_method_to_quaternion_orders branch September 17, 2023 11:18
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