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

sage.arith: Use sage.rings.abc and move some module-level imports into methods #32600

Closed
mkoeppe opened this issue Sep 30, 2021 · 9 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 30, 2021

(cherry-picked from #32432)

Depends on #32566

CC: @tscrim

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 5e5843a

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32600

@mkoeppe mkoeppe added this to the sage-9.5 milestone Sep 30, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

Changed dependencies from #32566: to #32566

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

New commits:

cee081asage.rings.real_mpfr.RealField_class: Inherit through a new class sage.rings.abc.RealField
99f4f10sage.rings.real_double.RealDoubleField_class: Inherit through a new class sage.rings.abc.RealDoubleField
29871d0ComplexField_class, ComplexDoubleField: Inherit through new classes sage.rings.abc.*
e1bb944src/sage/rings/polynomial/polynomial_element.pyx: Replace use of is_RealField by isinstance(sage.rings.abc.RealField) etc.
6e2c3c4src/sage/rings/abc.pyx: Add class docstrings
2bd66e4Merge #32566
4e2f0absrc/sage/arith/misc.py: move pari/flint imports into methods
5145d43src/sage/arith/misc.py: Use sage.rings.abc instead of importing element classes RealNumber, ComplexNumber
5e5843asage.arith.misc: Fixup

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

Commit: 5e5843a

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

Author: Matthias Koeppe

@tscrim
Copy link
Collaborator

tscrim commented Oct 2, 2021

comment:5

While it's not an issue here since it is not the main part of the algorithm, changes like this do incur a slight performance penalty:

-    if isinstance(z, (RealNumber, ComplexNumber)):
+    if isinstance(z.parent(), (RealField, ComplexField)):

This is just for future reference.

@tscrim
Copy link
Collaborator

tscrim commented Oct 2, 2021

Reviewer: Travis Scrimshaw

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 2, 2021

comment:6

Thanks! I agree with your analysis.

@vbraun
Copy link
Member

vbraun commented Oct 13, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants