Opened 3 years ago
Last modified 3 years ago
#22037 closed enhancement
Upgrade to Python 2.7.13 — at Version 17
Reported by:  jdemeyer  Owned by:  

Priority:  blocker  Milestone:  sage7.5 
Component:  packages: standard  Keywords:  
Cc:  SimonKing, hivert, slelievre, charpent  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/nthiery/upgrade_to_python_2_7_13 (Commits)  Commit:  b7bd4597e18af490dd9e6225e47b16d5ec8d79dc 
Dependencies:  #19735  Stopgaps: 
Description (last modified by )
Python 2.7.13rc1 leads to problems with sage related to __new__
. It is likely related to this change that was included in Python:
https://bugs.python.org/issue5322
See the following discussion on sagedevel: https://groups.google.com/forum/#!topic/sagedevel/IbugChsM26E
See also https://bugs.python.org/issue25731, where a patch at the same location was reverted as backward incompatible after causing a failure in Sage.
Note: upgrade to Python 2.7.12 done in #19735.
Change History (18)
comment:1 Changed 3 years ago by
 Description modified (diff)
comment:2 Changed 3 years ago by
 Description modified (diff)
comment:3 Changed 3 years ago by
 Description modified (diff)
Changed 3 years ago by
comment:4 followups: ↓ 5 ↓ 9 Changed 3 years ago by
So we have
class UniqueRepresentation(CachedRepresentation, WithEqualityById)
where CachedRepresentation
uses ClasscallMetaclass
and WithEqualityById
is a cdef class.
If I change that to
class UniqueRepresentation(WithEqualityById, CachedRepresentation)
the error goes away. I don't know if that would be a correct fix though.
comment:5 in reply to: ↑ 4 Changed 3 years ago by
Replying to thansen:
So we have
class UniqueRepresentation(CachedRepresentation, WithEqualityById)
where
CachedRepresentation
usesClasscallMetaclass
andWithEqualityById
is a cdef class.If I change that to
class UniqueRepresentation(WithEqualityById, CachedRepresentation)
the error goes away. I don't know if that would be a correct fix though.
In principle, the features provided by the two super classes are orthogonal (equality vs construction). Right now, I don't remember enough the details to see whether there could be subtleties due to one class being cdef. I'll dig further.
Simon, any opinion?
Cheers,
Nicolas
comment:6 Changed 3 years ago by
 Cc SimonKing added
comment:7 Changed 3 years ago by
 Description modified (diff)
comment:8 Changed 3 years ago by
Indeed Juliens example from https://bugs.python.org/issue25731 also exposes this issue:
$ cat foo.pxd cdef class B: cdef object b $ cat foo.pyx cdef class A: pass cdef class B: def __init__(self, b): self.b = b $ cat bar.py from foo import A, B class C(A, B): def __init__(self): B.__init__(self, 1) C() $ cython foo.pyx && gcc I/usr/include/python2.7 Wall shared fPIC o foo.so foo.c $ python c 'import bar' Traceback (most recent call last): File "<string>", line 1, in <module> File "bar.py", line 7, in <module> C() TypeError: foo.A.__new__(C) is not safe, use foo.B.__new__()
comment:9 in reply to: ↑ 4 Changed 3 years ago by
Replying to thansen:
So we have
class UniqueRepresentation(CachedRepresentation, WithEqualityById)
where
CachedRepresentation
usesClasscallMetaclass
andWithEqualityById
is a cdef class.If I change that to
class UniqueRepresentation(WithEqualityById, CachedRepresentation)
the error goes away. I don't know if that would be a correct fix though.
Looking back at the code, this still feels rather harmless.
I ran all long Sage tests; beside trivial doctests failures due to mro printouts, the only errors all boil down to the following:
sage t long src/sage/combinat/root_system/cartan_type.py ********************************************************************** File "src/sage/combinat/root_system/cartan_type.py", line 2999, in sage.combinat.root_system.cartan_type.CartanType_simple_finite.__setstate__ Failed example: unpickle_build(si1, {'tools':pg_unpickleModule('sage.combinat.root_system.type_A'), 't':['A', si2], 'letter':'A', 'n':si2}) Exception raised: Traceback (most recent call last): File "/opt/sagegit2/local/lib/python2.7/sitepackages/sage/combinat/root_system/cartan_type.py", line 3014, in __setstate__ self.__class__ = T.__class__ TypeError: __class__ assignment: 'CartanType_simple_finite' object layout differs from 'CartanType'
This is in a stub class whose only purpose is to support unpickling
rather old pickles (Sage <= 4.0). It's a stub for an old class that
does not exist anymore; upon unpickling an object, the set_state
method substitutes its stub class by the appropriate new class. This
is unsurprisingly fragile. I am unsure why it did not fail before and
fails now.
In any cases, making the stub class derive from UniqueRepresentation
ensures that the two classes have the same layout, which is slightly
cleaner than it was earlier. So that sounds like an acceptable fix.
I am about to push a branch with this fix and trivially updated doctests where needed. Let's see if the patchbot confirms that all test pass.
Still would be happy to have feedback from Simon.
Cheers,
Nicolas
comment:10 Changed 3 years ago by
 Branch set to u/nthiery/upgrade_to_python_2_7_13
comment:11 Changed 3 years ago by
 Commit set to b7bd4597e18af490dd9e6225e47b16d5ec8d79dc
Note: not being super familiar with the process, the above branch does *not* do the upgrade to Python 2.7.13; just the workaround suggested by Tobbias.
New commits:
8968314  22037: switch order of the bases of the class UniqueRepresentation

b7bd459  22037: more robust backwardcompatibility stub class CartanType_simple_finite

comment:12 Changed 3 years ago by
There are several places that need to be changed. The problem is that the errors are discovered one by one. Is there a standard way to rebuild just a single .pyx file? Right now I have to do a rebuild of sagelib everytime I change one. This is my patch right now, and there are still new errors coming up:
 a/src/sage/structure/unique_representation.py +++ b/src/sage/structure/unique_representation.py @@ 1176,7 +1176,7 @@ return cls(*args, **keywords) class UniqueRepresentation(CachedRepresentation, WithEqualityById): +class UniqueRepresentation(WithEqualityById, CachedRepresentation): r""" Classes derived from UniqueRepresentation inherit a unique representation behavior for their instances.  a/src/sage/categories/cartesian_product.py +++ b/src/sage/categories/cartesian_product.py @@ 18,7 +18,7 @@ native_python_containers = set([tuple, list, set, frozenset]) class CartesianProductFunctor(CovariantFunctorialConstruction, MultivariateConstructionFunctor): +class CartesianProductFunctor(MultivariateConstructionFunctor, CovariantFunctorialConstruction): """ A singleton class for the Cartesian product functor.  a/src/sage/rings/infinity.py +++ b/src/sage/rings/infinity.py @@ 544,7 +544,7 @@ class UnsignedInfinityRing_class(Singleton, Ring):  + __new__ = Ring.__new__ def __init__(self): """ Initialize ``self``. @@ 950,6 +950,7 @@ pass class InfinityRing_class(Singleton, Ring): + __new__ = Ring.__new__ def __init__(self): """ Initialize ``self``.  a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ 314,7 +314,7 @@ w.append((coeff,last)) self._data = w class FormalSums(UniqueRepresentation, Module): +class FormalSums(Module, UniqueRepresentation): """ The Rmodule of finite formal sums with coefficients in some ring R.  a/src/sage/misc/inherit_comparison.pyx +++ b/src/sage/misc/inherit_comparison.pyx @@ 83,5 +83,5 @@ t.tp_compare = b.tp_compare super(InheritComparisonMetaclass, self).__init__(*args) class InheritComparisonClasscallMetaclass(InheritComparisonMetaclass, ClasscallMetaclass): +class InheritComparisonClasscallMetaclass(ClasscallMetaclass, InheritComparisonMetaclass): pass  a/src/sage/rings/polynomial/polynomial_ring.py +++ b/src/sage/rings/polynomial/polynomial_ring.py @@ 1439,7 +1439,7 @@ return self._monics_max( max_degree ) raise ValueError("you should pass exactly one of of_degree and max_degree") class PolynomialRing_commutative(PolynomialRing_general, commutative_algebra.CommutativeAlgebra): +class PolynomialRing_commutative(commutative_algebra.CommutativeAlgebra, PolynomialRing_general): """ Univariate polynomial ring over a commutative ring. """  a/src/sage/rings/real_arb.pyx +++ b/src/sage/rings/real_arb.pyx @@ 355,7 +355,7 @@ if _do_sig(myprec): sig_off() return 0 class RealBallField(UniqueRepresentation, Field): +class RealBallField(Field, UniqueRepresentation): r""" An approximation of the field of real numbers using midrad intervals, also known as balls.
comment:13 Changed 3 years ago by
Ouch, this is getting more annoying. Let's see how big the list grows. It's more worrying to have to move UniqueRepresentation? to second place.
sage b
rebuilds just the Sage library. This is usually good enough when only a single pyx file needs to be rebuilt.
Have a good night,
comment:14 Changed 3 years ago by
Is better not to change the order of base classes and instead just define __new__
everywhere like I did for example for InfinityRing_class?
comment:15 Changed 3 years ago by
Ah, I had missed that. If this works, this would be acceptable as a temporary workaround. My first guess is that this would break the CachedRepresentation?; but I'll give it a try.
Anyone available for making a branch with Python 2.7.13, so that we can all easilly experiment with it? Otherwise, I'll try to squeeze this sometime today, but really no promise.
Thanks Tobias for investigating this hard!
comment:16 Changed 3 years ago by
 Cc hivert added
comment:17 Changed 3 years ago by
 Cc slelievre added
 Description modified (diff)
#19735 "Upgrade to Python 2.7.12" was just closed.
This is a small example showing how the ClasscallMetaclass? leads to the error.