Python 2.7.13rc1 breaks UnknownClass

[tha...@debian.org]

Hi sage-devel,

we're almost ready to upload Sage to Debian (in fact we basically have to upload it this week to make sure it's included in the next Debian release).

However, on Sunday python 2.7.13rc1 was uploaded to Debian and now we are facing a bug that I didn't quite manage to work around yet and that blocks any development at the moment.

This happens whenever Unknown is imported, meaning during the docbuild and when starting sage:

...
    from sage.misc.unknown import Unknown
  File "<BUILDDIR>/usr/lib/python2.7/dist-packages/sage/misc/unknown.py", line 164, in <module>
    Unknown = UnknownClass()
  File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (<CYTHONIZED>/sage/misc/classcall_metaclass.c:1413)
  File "sage/misc/cachefunc.pyx", line 1059, in sage.misc.cachefunc.CachedFunction.__call__ (<CYTHONIZED>/sage/misc/cachefunc.c:6080)
  File "<BUILDDIR>/usr/lib/python2.7/dist-packages/sage/structure/unique_representation.py", line 1022, in __classcall__
    instance = typecall(cls, *args, **options)
  File "sage/misc/classcall_metaclass.pyx", line 497, in sage.misc.classcall_metaclass.typecall (<CYTHONIZED>/sage/misc/classcall_metaclass.c:1862)
TypeError: sage.misc.fast_methods.WithEqualityById.__new__(UnknownClass) is not safe, use object.__new__()

I'm pretty sure it's caused by the change of https://bugs.python.org/issue5322
which is included in python 2.7.13rc1.

I hope you can help me to fix this, or at least provide a workaround.

Best,
Tobias

[François Bissey]

What did you try so far?
The most obvious thing to try, as far as I can see, is to add
__new__ = object.__new__()
before
def __init__(self):
in sage/misc/unknown.py

Francois

[Tobias Hansen]

I created a minimal cython example with classes and cdef classes that
inherit from each other in the same way as here to see if this is caused
by WithEqualityById being a cdef class. In the example everything worked
as it should.

Just because it appears in the error message I also tried replacing the two
(<PyTypeObject*>type).tp_call(cls, args, kwds)
in
sage/misc/classcall_metaclass.pyx
by
type.__call__(cls, *args, **kwds).

That didn't help either.

I'll try your suggestion next, thanks.

Best, Tobias

[tha...@debian.org]

When setting __new__ = object.__new__() for UnknownClass it goes on to the next similar error:

   ...
   File "<BUILDDIR>/usr/lib/python2.7/dist-packages/sage/categories/sets_cat.py", line 2752, in <module>
     cartesian_product = CartesianProductFunctor()

   File "sage/misc/classcall_metaclass.pyx", line 330, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (<CYTHONIZED>/sage/misc/classcall_metaclass.c:1413)
   File "sage/misc/cachefunc.pyx", line 1059, in sage.misc.cachefunc.CachedFunction.__call__ (<CYTHONIZED>/sage/misc/cachefunc.c:6080)
   File "<BUILDDIR>/usr/lib/python2.7/dist-packages/sage/structure/unique_representation.py", line 1022, in __classcall__
     instance = typecall(cls, *args, **options)
   File "sage/misc/classcall_metaclass.pyx", line 497, in sage.misc.classcall_metaclass.typecall (<CYTHONIZED>/sage/misc/classcall_metaclass.c:1862)

 TypeError: sage.misc.fast_methods.WithEqualityById.__new__(CartesianProductFunctor) is not safe, use sage.categories.functor.Functor.__new__()

[Francois Bissey]

I am guessing we should something more fundamental in
sage/structure/sage_object.pyx
since we are dealing with SageObject classes.
But I am not sure how to do the
__new__ = object.__new__ in cython.

François

[Jeroen Demeyer]

On 2016-12-07 04:17, Francois Bissey wrote:
> But I am not sure how to do the
> __new__ = object.__new__ in cython.

You certainly cannot do that since Cython's __new__ does non-trivial
stuff like setting the vtab for c(p)def method calls. This is also
partially the reason why cannot even implement __new__ for Cython cdef
classes.

[Francois Bissey]

OK, unless you can do something at the level
of tp_call (slot_tp_call may be) like Tobias
tried, that makes it a fundamental cython
problem. That kind of stuff will likely
happen with any cython cdef classes, right?

François

[Tobias Hansen]

That's what I tried with my cython example (attached), but there the
error didn't show.

Tobias

[Tobias Hansen]

I also just ran the Cython test suite with Python 2.7.13rc1 and there
were no problems.

Tobias

[Jeroen Demeyer]

What is the problem really? Neither from reading the bug report nor from
reading this thread do I understand what exactly changed in Python
2.7.13 causing this breakage.

[Jeroen Demeyer]

I created a ticket for this upgrade:
https://trac.sagemath.org/ticket/22037

But we should first upgrade to Python 2.7.12 before we can tackle 2.7.13:
https://trac.sagemath.org/ticket/19735

Anyone willing to work on the Python 2.7.12 upgrade? It would at least
make it easier to debug the problems with 2.7.13.

[Tobias Hansen]

If you look at the patch [1] that was applied, there is a new check for
tp_new(). I'm not 100% sure that's what causes the problem, but it looks
suspicious.


[1] https://bugs.python.org/file45526/update_one_slot2-2.7.patch

[Nicolas M. Thiery]

This seems very similar to the issue that Julien Cristau (in CC) had
stumbled into this Spring while trying to run Sage on top of a then
recent Python. That version of Python included a patch at exactly the
same line in Objects/typeobject.c. Julien argued that this patch was
backward incompatible and it got reverted:

https://bugs.python.org/issue25731

It seems it has come back in a new form. Julien: any opinion on that
new form? Is it a backward incompatible patch in Python that could be
argued to be reverted / fixed? If yes, is there any chance to make
this happen given the extremely tight schedule that we have?

I am going to dig in further tomorrow on the Sage side in search for a
workaround. As usual, the more eyeballs, the better.

Cheers,
Nicolas

PS: thanks Tobias for the ping!

--
Nicolas M. Thiéry "Isil" <nth...@users.sf.net>
http://Nicolas.Thiery.name/

[Tobias Hansen]

Yeah, it would be good to know if this is a regression in Python or a
bug in Sage. If it's considered a Python regression we could try to get
the change reverted in the Python Debian package.

Of course a workaround in Sage would be even better, because that is
fully under our control.

Best,
Tobias