Define behaviour of < and > for fractional ideals in a quaternion algebra#37113
Conversation
…uaternion algebra over QQ "smaller than" now means "included in"
dcoudert
left a comment
There was a problem hiding this comment.
a few comments on the coding style.
src/sage/modular/quatalg/brandt.py
Outdated
| verbose("found %s of %s ideals" % (len(ideals), self.dimension()), level=2) | ||
| if len(ideals) >= self.dimension(): | ||
| ideals = tuple(sorted(ideals)) | ||
| #order by basis matrix (as ideals were previously ordered) for backward compatibility and deterministic order of the output |
There was a problem hiding this comment.
Comments need a space after # and are usually aligned in 80 columns mode
- #order by basis matrix (as ideals were previously ordered) for backward compatibility and deterministic order of the output
+ # order by basis matrix (as ideals were previously
+ # ordered) for backward compatibility and
+ # deterministic order of the output| True | ||
| sage: O >= O | ||
| True | ||
|
|
There was a problem hiding this comment.
You can remove this empty line.
| sage: I != I # indirect doctest | ||
| False | ||
|
|
||
| TESTS: |
There was a problem hiding this comment.
TESTS: -> TESTS::
and add en empty line after TESTS::
|
Thank you for the explanation, I added your suggestions. |
|
Documentation preview for this PR (built with commit b47a604; changes) is ready! 🎉 |
|
I am not certain what to to about the failing tests: The first one failed in a completely unrelated file (src/sage_setup/clean.py) and I cannot reproduce the test failure locally. The second says that the empty line between examples and tests is not covered by any test. However, I that this line seems necessary since other functions using examples and tests have it also and removing it causes the test parser to fail. |
|
Indeed, these look like flukes. (The |
sagemathgh-37113: Define behaviour of < and > for fractional ideals in a quaternion algebra Previously, comparison of quaternion algebra fractional ideals (in a quaternion algebra over QQ) was documented only for equality and did compare the matrices under HNF of bases of the ideals. This gave strange and undocumented behavior when comparing two ideals with inequalities. This comparison is now replaced by the comparison of the fractional ideals as free modules. Now "smaller than" means "included in". #sd123 URL: sagemath#37113 Reported by: syndrakon Reviewer(s): David Coudert, Lorenz Panny
Previously, comparison of quaternion algebra fractional ideals (in a quaternion algebra over QQ) was documented only for equality and did compare the matrices under HNF of bases of the ideals. This gave strange and undocumented behavior when comparing two ideals with inequalities.
This comparison is now replaced by the comparison of the fractional ideals as free modules.
Now "smaller than" means "included in".
#sd123