Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#26319 closed defect (duplicate)

py3: patch six.add_metaclass to preserve wrapped class's __qualname__

Reported by: embray Owned by:
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: python3 Keywords:
Cc: chapoton Merged in:
Authors: Erik Bray Reviewers:
Report Upstream: Completely fixed; Fix reported upstream Work issues:
Branch: u/embray/python3/ticket-26319 (Commits, GitHub, GitLab) Commit: dff8d2450314d684cf10f0f426d1c6bd46af3f0f
Dependencies: Stopgaps:

Status badges

Description (last modified by embray)

The current version of six.add_metaclass does not preserve the original class's __qualname__ attribute (as it is not normally stored in the class's __dict__):

>>> from six import add_metaclass
>>> class A:
...     class B:
...         pass
...
>>> A.B
<class '__main__.A.B'>
>>> class MyMeta(type): pass
...
>>> class A:
...     @add_metaclass(MyMeta)
...     class B:
...         pass
...
>>> A.B
<class '__main__.B'>

Upstream PR: https://github.com/benjaminp/six/pull/260

Change History (11)

comment:1 Changed 3 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/python3/ticket-26319
  • Commit set to dff8d2450314d684cf10f0f426d1c6bd46af3f0f
  • Status changed from new to needs_review

Here I just monkey-patch six.add_metaclass at Sage import time. This shouldn't be a problem, and allows us to avoid updating every existing import of six.add_metaclass.

I'm happy to update the imports instead of monkey-patch, but that carries with it the danger that if someone doesn't know to use sage.misc.six.add_metaclass they could repeat this mistake.

Alternatively, we replace all imports of six with sage.misc.six and use only the latter. That will require a bit of fancy-footwork to make six.moves work but it's doable.


New commits:

dff8d24py3: Trac #26319: monkey-patch six.add_metaclass to fix support for nested

comment:2 Changed 3 years ago by embray

  • Description modified (diff)
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

comment:3 Changed 3 years ago by embray

  • Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

comment:4 Changed 3 years ago by embray

  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

comment:5 Changed 2 years ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

comment:6 Changed 2 years ago by embray

  • Milestone changed from sage-8.5 to sage-8.7

Retargeting some of my tickets (somewhat optimistically for now).

comment:7 Changed 2 years ago by embray

  • Cc chapoton added

I almost forgot about this ticket, but I believe it will still fix some tests on Python 3.

comment:8 Changed 2 years ago by jdemeyer

  • Milestone changed from sage-8.7 to sage-duplicate/invalid/wontfix
  • Report Upstream changed from Fixed upstream, but not in a stable release. to Completely fixed; Fix reported upstream
  • Resolution set to duplicate
  • Status changed from needs_review to closed

This is fixed by six 1.12.0, upgrade at #26969.

comment:9 Changed 2 years ago by embray

There still seem to be some test failures on Python 3 related to this, but I can confirm that six 1.12.0 at least contains the basic fix for nested classes with add_metaclass.

comment:10 Changed 2 years ago by jdemeyer

Note that six also has with_metaclass. Presumably that needs to be patched too?

comment:11 Changed 2 years ago by jdemeyer

I have a PR to fix two other bugs with with_metaclass at https://github.com/benjaminp/six/pull/211 but without feedback from upstream.

Note: See TracTickets for help on using tickets.