Opened 3 years ago

Closed 3 years ago

#28392 closed defect (fixed)

dynamic_class: ignore Python 3's new __weakref__ class attribute

Reported by: nthiery Owned by:
Priority: major Milestone: sage-9.0
Component: python3 Keywords:
Cc: chapoton, tscrim, embray Merged in:
Authors: Nicolas M. Thiéry Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 8046f1b (Commits, GitHub, GitLab) Commit: 8046f1b03a38c34cdb14cd6baa49b35e07b03359
Dependencies: Stopgaps:

Status badges

Description (last modified by embray)

Python3 introduce a new attribute __weakref__ in the datastructure for classes in addition to __dict__. As such, this attribute should not be copied over by dynamic_class. Otherwise we get errors, typically upon introspection:

    sage: from inspect import getmembers
    sage: c = EllipticCurve([0,0,1,-1,0])
    sage: getmembers(c)
    ---------------------------------------------------------------------------
    TypeError                                 Traceback (most recent call last)
    <ipython-input-5-bef5f5512029> in <module>()
    ----> 1 getmembers(c)

    /opt/sage-git/local/lib/python3.7/inspect.py in getmembers(object, predicate)
    339         # looking in the __dict__.
    340         try:
--> 341             value = getattr(object, key)
    342             # handle the duplicate key
    343             if key in processed:

    TypeError: descriptor '__weakref__' for 'Sets.ParentMethods' objects doesn't apply to 'EllipticCurve_rational_field_with_category' object

The same error can be triggered by calling directly c.__weakref__.

This can be reduced down to:

        sage: class A:
        ....:     pass
        sage: A.__weakref__
        <attribute '__weakref__' of 'A' objects>
        sage: Foo1 = sage.structure.dynamic_class.dynamic_class("Foo", (), A)
        sage: "__weakref__" in Foo1.__dict__
        True

(should be False).

This ticket fixes this.

Change History (8)

comment:1 Changed 3 years ago by nthiery

  • Branch set to u/nthiery/dynamic_class__do_not_copy_over_python_3_s_new___weakref___class_attribute

comment:2 Changed 3 years ago by nthiery

  • Commit set to 8046f1b03a38c34cdb14cd6baa49b35e07b03359
  • Status changed from new to needs_review
  • Summary changed from dynamic_class: do not copy over Python 3's new __weakref__ class attribute to dynamic_class: ignore Python 3's new __weakref__ class attribute

New commits:

8046f1b28392: dynamic_class: ignore Python 3's new __weakref__ class attribute

comment:3 Changed 3 years ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

LGTM

comment:4 Changed 3 years ago by embray

  • Description modified (diff)

comment:5 Changed 3 years ago by embray

  • Description modified (diff)

comment:6 Changed 3 years ago by embray

(FWIW __weakref__ is not new, just this error, but the fact it was being copied is a mistake)

comment:7 Changed 3 years ago by chapoton

  • Milestone changed from sage-8.9 to sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:8 Changed 3 years ago by vbraun

  • Branch changed from u/nthiery/dynamic_class__do_not_copy_over_python_3_s_new___weakref___class_attribute to 8046f1b03a38c34cdb14cd6baa49b35e07b03359
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.