Opened 6 years ago

Last modified 5 years ago

#21636 new defect

Import cycles in Sage

Reported by: Erik Bray Owned by:
Priority: major Milestone: sage-8.0
Component: refactoring Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #22752 Stopgaps:

Status badges

Description

I've actually had this problem for a long time, but never got around to reporting it--importing several modules in sage results in an ImportError. For example:

>>> import sage.rings.integer_ring
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "sage/rings/integer.pxd", line 7, in init sage.rings.integer_ring (/home/embray/src/sagemath/sage/src/build/cythonized/sage/rings/integer_ring.c:14483)
  File "sage/rings/rational.pxd", line 8, in init sage.rings.integer (/home/embray/src/sagemath/sage/src/build/cythonized/sage/rings/integer.c:48888)
  File "sage/rings/fast_arith.pxd", line 3, in init sage.rings.rational (/home/embray/src/sagemath/sage/src/build/cythonized/sage/rings/rational.c:36668)
  File "sage/libs/pari/gen.pxd", line 6, in init sage.rings.fast_arith (/home/embray/src/sagemath/sage/src/build/cythonized/sage/rings/fast_arith.c:8273)
  File "sage/libs/pari/gen.pyx", line 90, in init sage.libs.pari.gen (/home/embray/src/sagemath/sage/src/build/cythonized/sage/libs/pari/gen.c:134407)
  File "/home/embray/src/sagemath/sage/local/lib/python2.7/site-packages/sage/rings/infinity.py", line 227, in <module>
    from sage.rings.integer_ring import ZZ
ImportError: cannot import name ZZ

This demonstrates the cycle directly: sage.rings.integer_ring -> sage.rings.rational -> sage.rings.fast_arith -> sage.libs.pari.gen -> sage.rings.infinity -> sage.rings.integer_ring

but the same crash can be invoked indirectly by almost anything that imports sage.rings.integer_ring.

Making sure to import sage.all first works around it, since I guess it makes sure to import modules in a very specific order so that this doesn't occur. But that shouldn't be necessary--it should be possible to directly import modules without failure. Either the import cycle should be broken, or it should be handled automatically at import time (such as in sage.rings.__init__) rather than having to import the all module manually.

Change History (15)

comment:1 Changed 6 years ago by Erik Bray

Component: miscrefactoring

comment:2 Changed 6 years ago by Jeroen Demeyer

This is a much more general problem: imports in Sage are a huge can of worms: they have to be done in a very specific order or they break.

We often try to work around this problem by importing inside methods/functions:

def foo():
    import something
    something()

instead of

import something

def foo():
    something()

or various tricks with lazy/late imports.

And Cython makes it even harder than it would otherwise be, since Cython's imports are always done first, before anything else.

comment:3 Changed 6 years ago by Erik Bray

I kind of figured that was the case--even if I resolved this one I wouldn't be surprised if there were others. Still, it should be fixed/worked around where found because it otherwise makes Sage unusable as a "normal" library.

comment:4 in reply to:  3 Changed 6 years ago by Jeroen Demeyer

Replying to embray:

I wouldn't be surprised if there were others.

I am sure that there are others, possibly many.

Still, it should be fixed/worked around where found because it otherwise makes Sage unusable as a "normal" library.

If you remember to import sage.all first, it's not such a big deal. In fact, external packages should import directly from sage.all if possible (from sage.all import ZZ).

comment:5 Changed 6 years ago by Erik Bray

Yes, but I don't think that's a valid solution.

comment:6 Changed 6 years ago by Erik Bray

I guess more precisely what I'm saying is that sage.all (and the related submodules) are serving a double purpose. On one hand they're importing nearly everything and setting up the global namespace that one expects to find in the sage repl. On the other hand they're also doing some critical setup required for modules to even work properly (such as carefully ordered imports and some other things).

The latter stuff should happen in __init__ modules, which should only import what they need to do that setup and nothing else.

comment:7 Changed 6 years ago by Jeroen Demeyer

Summary: Import cycle in sage.rings.integer_ringImport cycles in Sage

comment:8 Changed 6 years ago by Erik Bray

I'd still be interested in working on improving the situation here, but it's pretty low priority right now.

comment:9 in reply to:  8 Changed 5 years ago by Jeroen Demeyer

Milestone: sage-7.4sage-8.0
Priority: minormajor

Replying to embray:

I'd still be interested in working on improving the situation here, but it's pretty low priority right now.

I'd like to push the priority on this one. #22747 totally breaks down when import cycles are involved because Cython does things in a different order with #22747.

Do you know of a structural solution to breaking import cycles? I'm afraid it will be a lot of work by hand to move things around.

comment:10 Changed 5 years ago by Erik Bray

Definitely a lot of work by hand to move things around. Would you like me to take a stab at it?

One thing that is ugly, but that you can often get pretty far with, is moving some imports to the bottom of the module. I'm not sure how that will play with Cython though.

comment:11 in reply to:  10 Changed 5 years ago by Jeroen Demeyer

Replying to embray:

One thing that is ugly, but that you can often get pretty far with, is moving some imports to the bottom of the module. I'm not sure how that will play with Cython though.

For Cython that doesn't matter. Cython mostly ignores the order of the source files, it does things in its own order.

My plan is to use lazy imports a lot more. I created #22751 to improve support for lazy imports in Cython code.

comment:12 Changed 5 years ago by Erik Bray

How exactly do lazy imports work in Sage? Is there a custom module subclass? I never really looked into it much.

comment:13 in reply to:  12 Changed 5 years ago by Jeroen Demeyer

Replying to embray:

How exactly do lazy imports work in Sage? Is there a custom module subclass? I never really looked into it much.

I'm currently working on improving lazy imports. I have an implementation at #22752 which involves changing builtins.__import__.

comment:14 Changed 5 years ago by Jeroen Demeyer

Dependencies: #22752

comment:15 Changed 5 years ago by Jeroen Demeyer

Good news for myself: I think I can get #22747 to work without requiring this.

Note: See TracTickets for help on using tickets.