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:

Status badges

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 rws

  • Branch set to u/rws/circular_import_in_maxima_lib_py

comment:2 Changed 7 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to a3bc86c0855155e2755f9174f95f8dd578827d92
  • Status changed from new to needs_review

New commits:

a3bc86c16520: fix whole file imports

comment:3 Changed 7 years ago by 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)

comment:4 Changed 7 years ago by 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

Last edited 7 years ago by nbruin (previous) (diff)

comment:5 Changed 7 years ago by rws

  • 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 vbraun

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.