sagemath/sage

Remove .all import of infinity

Closed this issue · 23 comments

... throughout the Sage library.

sage.rings is becoming a namespace package in the course of modularization (#29705), so we are replacing imports from sage.rings.all throughout the Sage library.

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 97861cd

Reviewer: Kwankyu Lee

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

Author: Matthias Koeppe

Commit: 2446ec9

New commits:

2446ec9git grep -l 'import infinity *$' | xargs sed -i.bak 's/ sage.*all import infinity *$/ sage.rings.infinity import infinity/'

Changed commit from 2446ec9 to bc2d970

Branch pushed to git repo; I updated commit sha1. New commits:

9b83d92src/sage/combinat: Remove all-import of infinity
0224b35src/sage/rings/valuation: Remove all-import of infinity
3c5750csrc/sage/modular/abvar: Remove all-import of infinity
aa797a5src/sage/rings/polynomial: Remove all-import of infinity
bb3ca7asrc/sage/modular/modform_hecketriangle: Remove all-import of infinity
8e096f4src/sage/modular/modsym: Remove all-import of infinity
58eb1f9src/sage/rings/valuation/valuation_space.py: Remove all-import of infinity
bc2d970src/sage/schemes/elliptic_curves/height.py: Remove all-import of infinity

Branch pushed to git repo; I updated commit sha1. New commits:

c658a23src/sage/combinat/cluster_algebra_quiver/quiver_mutation_type.py: Fix typo

Changed commit from bc2d970 to c658a23

Branch pushed to git repo; I updated commit sha1. New commits:

97861cdsrc/sage/rings/valuation/valuation_space.py: Fix typo

Changed commit from c658a23 to 97861cd

Description changed:

--- 
+++ 
@@ -1 +1,4 @@
+... throughout the Sage library.
 
+`sage.rings` is becoming a namespace package in the course of modularization (#29705), so we are replacing imports from `sage.rings.all` throughout the Sage library.
+
comment:6

The failure in src/sage/rings/integer.pyx is not from this ticket.

comment:7

What is the new policy? Do we intend to remove all all.py eventually? If not, what should(can) remain in all.py? Or this is determined by how we split sage library? The last case is not good, I think...

Or there won't be no change in all.py but we stop importing things from it in Sage library?

comment:8

All all.py are kept, for the purpose of populating the global interactive environment (sage.all).

We are just getting rid of imports from sage.PAC.KAGE.all within the Sage library. In particular, the imports from those packages sage.PAC.KAGE that become namespace packages, i.e., filled with modules from several distributions.

comment:9

I am asking because you keep lines like

from sage.rings.all import AA, QQbar, CC

comment:10

Just because I can't do everything on the same ticket. These will also go away, or perhaps it is already done on another ticket.

comment:11

CC, for example, is subject of #30741

comment:12

Replying to @mkoeppe:

Just because I can't do everything on the same ticket. These will also go away, or perhaps it is already done on another ticket.

Okay.

May I understand the new policy as that we never import from .all within the Sage library?

comment:13

That would be a safe and easy to understand way to phrase a new policy.

But that policy would be broader than what is technically needed. For example, I have no plans to split up sage.plot into several distributions (sage.plot will remain an ordinary package with __init__.py). So it does not matter whether library code imports from sage.plot.all or sage.plot.plot.

comment:14

These changes seem to increase startup time slightly. But I think we can allow this.

Reviewer: Kwankyu Lee

comment:15

Thanks for reviewing!