Circular imports: We have a problem

[Joachim Durchholz]

Hi all,

I've been working on removing C, and now that all the simple cases are
done, I'm getting to the harder ones - those where SymPy code is called
during the import (which will fail if the SymPy code being called has
not yet fully imported).

There's the solvable cases - stuff like ask.py's known-properties
database, where the initialization can be moved from module-level code
to a function, so the initialization will happen on demand, after the
"import sympy" statement has completed.

And now I'm at an unsolvable case: anything that uses S will run class
initialization code. And replacing that code with something delayed
would be impractical.

Here's the bottom end of such a stack trace:

File "sets/fancysets.py", line 173, in <module>
class Reals(with_metaclass(Singleton, Interval)):
File "core/compatibility.py", line 181, in __new__
return meta(name, bases, d)
File "core/singleton.py", line 70, in __init__
the_instance = ctor(cls)
File "sets/fancysets.py", line 176, in __new__
return Interval.__new__(cls, -S.Infinity, S.Infinity)
File "sets/sets.py", line 744, in __new__
if (end < start) == True:
File "core/numbers.py", line 1005, in __lt__
other = other.evalf()
File "core/evalf.py", line 1343, in evalf
_create_evalf_table()
File "core/evalf.py", line 1184, in _create_evalf_table
from sympy.functions.combinatorial.numbers import bernoulli
ImportError: cannot import name bernoulli

What happens is that the "Reals" class gets initialized, which is a
Singleton, so its Singleton metaclass runs the constructor to get "the
instance":
File "core/singleton.py", line 70, in __init__
the_instance = ctor(cls)
In the case of Reals, the constructor quite reasonably tries to define
th set of Reals as the Interval between -Infinity and Infinity.
Unfortunately, this runs SymPy code during class definition time, which
is during import time, at which time we can't rely on SymPy being fully
available yet. In this particular instance, the code runs into
_create_evalf_table, but minor restructuring could shift the actual
point of failure almost anywhere - all that's needed is an import that
runs some piece of SymPy code that isn't imported yet.


Question is: WHAT DO WE DO?
===========================

1) What *will* work is to replace the attribute with a function.
Attributes need to be initialized before use, functions don't need to
work until they are actually needed.
Except then S.Foo would have to be rewritten as S.Foo() for all values
of Foo. Anybody who ever used SymPy and picked up the S.Foo idiom for
his code will curse us to hell if we do that.

2) We could defer creation of the_instance. S would first simply collect
names for its namespace, and trigger instance creation on request (which
could be done from sympy.__init__.py).
Any uses of S at import time would fail and need to be fixed (except
those in the Singleton class constructors). I do not know how many files
would be affected by that -> not nice, but maybe an option.

3) We could change the code that accesses the_instance so that actual
construction is delayed until actual use.
the_instance is used only inside the __init__ function of Singleton:

class Singleton(ManagedProperties):
...
def __init__(cls, name, bases, dict_):
...
# The creation call that needs to be delayed:
the_instance = ctor(cls)
def __new__(cls):
# Easy: call ctor(cls) on the first __new__.
return the_instance
...
# Harder: route S's getattr to a function that
# runs ctor(cl) to construct the_instance.
setattr(S, name, the_instance)

I don't have a full grasp of what ManagedProperties does yet, so this
may or may not be doable.


Questions? Comments? More ideas how to deal with this?

[Aaron Meurer]

I think ideally we should avoid calls like this being run at import
time. To me, this indicates a design issue in the sets.

Aaron Meurer

> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sympy+un...@googlegroups.com.
> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sympy.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/sympy/54FA0980.9070304%40durchholz.org.
> For more options, visit https://groups.google.com/d/optout.

[Joachim Durchholz]

Am 06.03.2015 um 21:34 schrieb Aaron Meurer:
>> Here's the bottom end of such a stack trace:
>>
>> File "sets/fancysets.py", line 173, in <module>
>> class Reals(with_metaclass(Singleton, Interval)):
>> File "core/compatibility.py", line 181, in __new__
>> return meta(name, bases, d)
>> File "core/singleton.py", line 70, in __init__
>> the_instance = ctor(cls)
>> File "sets/fancysets.py", line 176, in __new__
>> return Interval.__new__(cls, -S.Infinity, S.Infinity)
>> File "sets/sets.py", line 744, in __new__
>> if (end < start) == True:
>> File "core/numbers.py", line 1005, in __lt__
>> other = other.evalf()
>
> I think ideally we should avoid calls like this being run at import
> time. To me, this indicates a design issue in the sets.

This is the class in question (lines 173 ff in sets/fancysets.py):

class Reals(with_metaclass(Singleton, Interval)):

def __new__(cls):
return Interval.__new__(cls, -S.Infinity, S.Infinity)


So it needs Interval and Infinity in its constructor.
If there's a design problem with that code, I guess it's a pervasive one.

[Aaron Meurer]

Hmm, I didn't notice that the only reason this class is constructed at
import time is that it uses the Singleton metaclass.

So one of your suggestions is probably the best fix. Your suggestion 2
(or 3, I don't see the difference) sounds like the best plan to me. It
will break down if something at import time calls S.Whatever, but that
should be avoided anyway.

Even so, I'm still not a fan of evalf() being called inside a class
constructor. Pervasive use of evalf is one of the reasons SymPy is
slow.

Aaron Meurer

>
> --
> You received this message because you are subscribed to the Google Groups
> "sympy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sympy+un...@googlegroups.com.
> To post to this group, send email to sy...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sympy.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/sympy/54FA1AEA.2010209%40durchholz.org.

[Joachim Durchholz]

Am 06.03.2015 um 23:12 schrieb Aaron Meurer:
> So one of your suggestions is probably the best fix. Your suggestion 2
> (or 3, I don't see the difference) sounds like the best plan to me.

The plan was to have 2: Set up S.Whatever but delay instance creation,
and 3: delay S.Whatever creation.
Somehow things got mixed up while I wrote the suggestions. Right now I
think it doesn't matter to callers so it's not important which of the
two it will be.

> It
> will break down if something at import time calls S.Whatever, but that
> should be avoided anyway.

Ack.

> Even so, I'm still not a fan of evalf() being called inside a class
> constructor. Pervasive use of evalf is one of the reasons SymPy is
> slow.

I can see that, but calling evalf() inside a singleton's constructor
should be okay, by the definition of "singleton".

Originally, I was a bit scared at the prospect of adding delayed
attribute initialization to S. However, some more investigation turned
up a route that I feel confident with, so I'll try that.

[Joachim Durchholz]

Am 06.03.2015 um 23:12 schrieb Aaron Meurer:

> Hmm, I didn't notice that the only reason this class is constructed at
> import time is that it uses the Singleton metaclass.

Singleton actually explicitly documents that it's running the instance
constructor during class declaration:

> The class is instantiated immediately at the point where it is
> defined by calling cls.__new__(cls). This instance is cached and
> cls.__new__ is rebound to return it directly.

As I understand things, the behaviour that we want is the exact
opposite, namely that a singleton class defers constructing the
singleton until the first time it is explicitly requested.

So I'm changing that behaviour, and removing that section from the
docstring.


There's one funny thing I noticed: Singleton caches the original
constructor, under the name of _new_instance, so that subclasses can
still call it to construct their own instances.
However, _new_instance is never ever called in all of SymPy. There's not
even a unit test for that behaviour.
Should I remove that feature? It would simplify the code, not in terms
of lines of code but conceptually (because finding the constructor
involves looking in the MRO chain so anybody trying to understand the
code has to understand both the lookup and the caching at the same time).