circular import in maxima_lib.py

[rws]

The lazy import in #2516 uncovered bad imports in interfaces/maxima_lib.py. To enable fixing it independently of a review of that ticket a separate ticket is created.

[rws]

New commits:

​a3bc86c

16520: fix whole file imports

[nbruin]

Just out of curiosity, do you have a theory why using from ... import ... works better than import ...? I would normally expect that from ... import ... constructions are more fragile (although it probably doesn't matter if the imported symbols are accessed in the module initialization code anyway)

[nbruin]

I noticed these executed at runtime (as opposed to module initialization time):

+ from sage.rings.rational import Rational
+ from sage.rings.number_field.number_field_element_quadratic import NumberFieldElement_quadratic
+ if isinstance(obj, Rational):

That's bad, because these are global assignments. You don't want to do those again and again. Also, I find it hard to believe that those two modules somehow refer back to maxima_lib.

The following runs at module initialization time anyway. I don't see how `from ... makes this better than import sage.symbolic.pynac`. It's not critical, so I don't really care, apart from the fact that the namespace gets an extra unnecessary symbol symbol_table.

+from sage.symbolic.pynac import symbol_table
+max_to_pynac_table = symbol_table['maxima']

In general, I have trouble seeing how maxima_lib can lead to circular imports, because maxima_lib is an interface and as such gets referenced by virtually no-one. The only place is sage.calculus.calculus and I've very carefully made sure we don't reference that. Furthermore, even there it's imported lazily, and for very good reason: Importing maxima_lib initializes ecl with maxima which takes considerable time.

So I suspect it's not so much a circular import but a bad interaction between the lazy import of maxima_lib and the attempted lazy import of hypergeometric. The proper solution is probably to *not* lazily import hypergeometric into maxima_lib, but import it plainly there. By the time you're importing maxima_lib you're already committed to significant initialization time, so unless hypergeometric does something really strange, I don't expect it will have a significant effect.

TL;DR: I'm not convinced that this ticket is actually solving a real problem.

addition: I think the following gives a strong indication that maxima_lib does not have a circular import. The following is supposed to be a complete list of the imports done in maxima_lib. A circular import would imply that one of those imports leads to importing maxima_lib, which means the LazyImport should resolve:

sage: from sage.symbolic.ring import SR, var
sage: from sage.interfaces.maxima_abstract import (MaximaAbstract, MaximaAbstractFunction,
....:   MaximaAbstractElement, MaximaAbstractFunctionElement,
....:   MaximaAbstractElementFunction)
sage: import sage.rings.real_double
sage: import sage.symbolic.expression
sage: import sage.functions.trig
sage: import sage.functions.log
sage: import sage.functions.other
sage: import sage.symbolic.integration.integral
sage: from sage.symbolic.operators import FDerivativeOperator
sage: from sage.symbolic.ring import is_SymbolicVariable
sage: type(sage.calculus.calculus.maxima)
<type 'sage.misc.lazy_import.LazyImport'>

This could just be a stale LazyImport proxy, but it's not. Note the runtime needed to compute the hash for the first time. This is indicating the ecl initialization is only run then.

sage: %time hash(sage.calculus.calculus.maxima)
CPU times: user 0.77 s, sys: 0.02 s, total: 0.79 s
Wall time: 0.80 s
-7971541566211231133

The symbol is now resolved and indeed, hash is instantaneous:

sage: type(sage.calculus.calculus.maxima)
<class 'sage.interfaces.maxima_lib.MaximaLib'>
sage: %time hash(sage.calculus.calculus.maxima)
CPU times: user 0.00 s, sys: 0.00 s, total: 0.00 s
Wall time: 0.00 s
-7971541566211231133

There are stale lazyimport proxies lying around:

sage: type(maxima_calculus)
<type 'sage.misc.lazy_import.LazyImport'>
sage: hash(maxima_calculus)
-7971541566211231133
sage: type(maxima_calculus)
<type 'sage.misc.lazy_import.LazyImport'>

That's now #16522

[rws]

Thanks for your thorough review. I assume this one can be closed as invalid then.