Metaclass syntax changed in Python 3

[3a5ab40a-5e15-47f8-8025-1665d51c0660]

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 meta-ticket ticket:16052.

REFERENCE: http://stackoverflow.com/questions/18513821/python-metaclass-understanding-the-with-metaclass

CC: @tscrim @jdemeyer

Component: python3

Author: Jeroen Demeyer

Branch/Commit: f1ca05f

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/16074

[jdemeyer]

Description changed:

--- 
+++ 
@@ -3,6 +3,6 @@

 But the code has to depend on the Python version!

-There are 30 effected modules. 
+There are 30 affected modules. 

 This ticket is tracked as a dependency of meta-ticket ticket:16052.

[fchapoton]

Description changed:

--- 
+++ 
@@ -6,3 +6,5 @@
 There are 30 affected modules. 

 This ticket is tracked as a dependency of meta-ticket ticket:16052.
+
+REFERENCE: http://stackoverflow.com/questions/18513821/python-metaclass-understanding-the-with-metaclass

[fchapoton]

comment:5

first half in #22474 (combinat folder)

[fchapoton]

comment:6

another easy step in #22479

then there remains more difficult cases

[jdemeyer]

comment:7

Cython files: #22537

[fchapoton]

comment:8

@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.

[jdemeyer]

comment:9

For reference, can you at least push the non-working branch?

[fchapoton]

comment:10

here it is

---

New commits:

c9bb275

metaclass tentative for clifford algebras

[fchapoton]

Branch: public/16074

[fchapoton]

Commit: c9bb275

[fchapoton]

comment:11

failing as follows

  File "/home/chapoton/sage/local/lib/python2.7/site-packages/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 (non-strict) subclass of the metaclasses of all its bases

[tscrim]

comment:12

@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.

[jdemeyer]

comment:13

The problem is the decorator, because it works in 2 steps:

1. The class is created the usual way.

2. The decorator modifies the class to have a new metaclass.

The problem is that step 1. already fails, so the decorator cannot do anything.

[jdemeyer]

comment:14

I'll look into this.

[jdemeyer]

comment:15

Replying to @fchapoton:

@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?

[fchapoton]

comment:16

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".

[jdemeyer]

comment:17

Hmm, I agree that it's not easy :-) I'm making some progress, but I'm not quite there yet.

[jdemeyer]

Author: Jeroen Demeyer

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from c9bb275 to 36009e4

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f61a1ae	Replace `__metaclass__` with six.with_metaclass
677cc66	Check for metaclass by checking types instead of __metaclass__
36009e4	six.with_metaclass: use type.__call__ instead of type.__new__

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b2beb04	six.with_metaclass: override `__call__` instead of __new__
b2108e6	Replace `__metaclass__` with six.with_metaclass
9635fb4	Check for metaclass by checking types instead of __metaclass__

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 36009e4 to 9635fb4

[jdemeyer]

comment:21

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.

[tscrim]

comment:22

I don't agree with this change (and subsequent changes from this):

diff --git a/src/sage/misc/six.py b/src/sage/misc/six.py
index 8273d91..5f03a96 100644
--- a/src/sage/misc/six.py
+++ b/src/sage/misc/six.py
@@ -8,7 +8,7 @@ from __future__ import absolute_import
 from six import *

-def with_metaclass(meta, *bases):
+def with_metaclass(*args):
     """
     Create a base class with a metaclass.

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.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

f1ca05f

Restore (meta, *bases) arguments for with_metaclass()

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 9635fb4 to f1ca05f

[jdemeyer]

comment:24

Replying to @tscrim:

I don't agree with this change

Fixed.

[jdemeyer]

comment:25

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.

[jdemeyer]

comment:26

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.

[tscrim]

comment:27

Makes sense to me. It's somewhat less pretty than the decorator, but this works for now and gets us closer to Python3.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

Changed branch from public/16074 to f1ca05f