Opened 7 years ago
Closed 7 years ago
#16520 closed defect (duplicate)
circular import in maxima_lib.py
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | interfaces | Keywords: | maxima, cleanup |
Cc: | Merged in: | ||
Authors: | Ralf Stephan | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/rws/circular_import_in_maxima_lib_py (Commits, GitHub, GitLab) | Commit: | a3bc86c0855155e2755f9174f95f8dd578827d92 |
Dependencies: | Stopgaps: |
Description
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.
Change History (6)
comment:1 Changed 7 years ago by
- Branch set to u/rws/circular_import_in_maxima_lib_py
comment:2 Changed 7 years ago by
- Commit set to a3bc86c0855155e2755f9174f95f8dd578827d92
- Status changed from new to needs_review
comment:3 Changed 7 years ago by
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)
comment:4 Changed 7 years ago by
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
comment:5 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-duplicate/invalid/wontfix
- Status changed from needs_review to positive_review
Thanks for your thorough review. I assume this one can be closed as invalid then.
comment:6 Changed 7 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
New commits:
16520: fix whole file imports