Opened 9 years ago
Closed 6 years ago
#16074 closed enhancement (fixed)
Metaclass syntax changed in Python 3
Reported by:  Wilfried Luebbe  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  python3  Keywords:  python3 
Cc:  Travis Scrimshaw, Jeroen Demeyer  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  f1ca05f (Commits, GitHub, GitLab)  Commit:  f1ca05f39db7d8ca8afc379808858c568619b9eb 
Dependencies:  Stopgaps: 
Description (last modified by )
The tool 2to3 changes the code to the new Py3 syntax.
But the code has to depend on the Python version!
There are 30 affected modules.
This ticket is tracked as a dependency of metaticket ticket:16052.
REFERENCE: http://stackoverflow.com/questions/18513821/pythonmetaclassunderstandingthewithmetaclass
Change History (28)
comment:1 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:2 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:3 Changed 7 years ago by
Component:  distribution → python3 

Description:  modified (diff) 
Summary:  Python 3 preparation: The metaclass syntax has changed in Py → Metaclass syntax changed in Python 3 
comment:4 Changed 6 years ago by
Description:  modified (diff) 

Milestone:  sage6.4 → sage7.6 
comment:5 Changed 6 years ago by
comment:6 Changed 6 years ago by
another easy step in #22479
then there remains more difficult cases
comment:8 followup: 15 Changed 6 years ago by
Cc:  Travis Scrimshaw Jeroen Demeyer added 

@tscrim: I have not been able to fix the use of metaclass in
* src/sage/algebras/clifford_algebra.py * src/sage/algebras/commutative_dga.py * src/sage/modular/modform_hecketriangle/graded_ring_element.py
The expected python3 replacement is
from six import add_metaclass @add_metaclass(name_of_metaclass) class something
But this does not work for the three mentioned files.
comment:10 Changed 6 years ago by
Branch:  → public/16074 

Commit:  → c9bb275589209c5da4b16c475c076f12a2557816 
Milestone:  sage7.6 → sage8.0 
comment:11 Changed 6 years ago by
failing as follows
File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/algebras/clifford_algebra.py", line 2163, in <module> class ExteriorAlgebraDifferential(ModuleMorphismByLinearity, UniqueRepresentation): TypeError: Error when calling the metaclass bases metaclass conflict: the metaclass of a derived class must be a (nonstrict) subclass of the metaclasses of all its bases
comment:12 Changed 6 years ago by
@add_metaclass
probably does something with the metaclass. Unless it is because of our heirarchy of metaclass...but I doubt it is that. It might be a decorator that needs to be added to the subclasses as well.
comment:13 Changed 6 years ago by
The problem is the decorator, because it works in 2 steps:
 The class is created the usual way.
 The decorator modifies the class to have a new metaclass.
The problem is that step 1. already fails, so the decorator cannot do anything.
comment:15 Changed 6 years ago by
Replying to chapoton:
@tscrim: I have not been able to fix the use of metaclass in
* src/sage/algebras/clifford_algebra.py * src/sage/algebras/commutative_dga.py * src/sage/modular/modform_hecketriangle/graded_ring_element.py
There are some other files with __metaclass__
, such as src/sage/structure/unique_representation.py
. Should I try to fix these on this ticket too or is there a different ticket?
comment:16 Changed 6 years ago by
Please fix the metaclass issue in all places where you can. There is no other ticket. I only listed the algebraic use cases. The rest is more like "sage infrastructure".
comment:17 Changed 6 years ago by
Authors:  → Jeroen Demeyer 

Hmm, I agree that it's not easy :) I'm making some progress, but I'm not quite there yet.
comment:18 Changed 6 years ago by
Commit:  c9bb275589209c5da4b16c475c076f12a2557816 → 36009e4e7fef4cf8136695279af1e8b17cc7cd5c 

comment:19 Changed 6 years ago by
Commit:  36009e4e7fef4cf8136695279af1e8b17cc7cd5c → 9635fb408451c3a6aca1d26deafed6461ac62f01 

comment:20 Changed 6 years ago by
Status:  new → needs_review 

comment:21 Changed 6 years ago by
Done. The main effort here is fixing sage.misc.six.with_metaclass
(the first of 3 commits).
After this ticket, there is no remaining Sage code using __metaclass__
. Some doctests still use __metaclass__
, but I suggest to fix that later.
comment:22 followup: 24 Changed 6 years ago by
I don't agree with this change (and subsequent changes from this):

src/sage/misc/six.py
diff git a/src/sage/misc/six.py b/src/sage/misc/six.py index 8273d91..5f03a96 100644
a b from __future__ import absolute_import 8 8 from six import * 9 9 10 10 11 def with_metaclass( meta, *bases):11 def with_metaclass(*args): 12 12 """ 13 13 Create a base class with a metaclass. 14 14
as later we require the first argument to be the meta class. So if someone happened to pass nothing, it would get an error message starting much later in the code. IMO, it also obfuscates the code a little bit too.
comment:23 Changed 6 years ago by
Commit:  9635fb408451c3a6aca1d26deafed6461ac62f01 → f1ca05f39db7d8ca8afc379808858c568619b9eb 

Branch pushed to git repo; I updated commit sha1. New commits:
f1ca05f  Restore (meta, *bases) arguments for with_metaclass()

comment:25 Changed 6 years ago by
I wanted to submit these fixes to with_metaclass
upstream to six
. However, when looking at the original six
code, I understood what was going wrong and it is really simple.
I also realized that it was #18503 which is the cause for the metaclass breakage. In other words, #18503 did the wrong thing and actually made everything more complicated.
It turns out that #18503 can be fixed in a much simpler way, see https://github.com/benjaminp/six/pull/191 and then this issue (#16074) simply does not occur.
comment:26 Changed 6 years ago by
That being said, I think the current solution on this ticket is structurally better than upstream's with_metaclass
: upstream is overriding __new__
with __call__
which is a bit fishy. Instead, I am overriding __call__
with __call__
which makes more sense.
comment:27 Changed 6 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
Makes sense to me. It's somewhat less pretty than the decorator, but this works for now and gets us closer to Python3.
comment:28 Changed 6 years ago by
Branch:  public/16074 → f1ca05f39db7d8ca8afc379808858c568619b9eb 

Resolution:  → fixed 
Status:  positive_review → closed 
first half in #22474 (combinat folder)