Opened 5 years ago

Last modified 2 years ago

#15648 new defect

lazy import attributes of a class are not substituted back after the import

Reported by: nthiery Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: misc Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

sage: class A:
....:     Associative = LazyImport('sage.categories.magmas', 'Magmas')
....: 
sage: A.Associative
<class 'sage.categories.magmas.Magmas'>
sage: type(A.__dict__["Associative"])
<type 'sage.misc.lazy_import.LazyImport'>

Change History (15)

comment:1 Changed 5 years ago by SimonKing

See my comment at #10963. We could simply create a lazy class attribute with an import statement inside. Proof of concept:

sage: def imported_lazy_class_attribute(module_name, cls_name):                         
....:     return lazy_class_attribute(lambda cls: getattr(__import__(module_name, {}, {}, [cls_name]),cls_name))
....: 
sage: class Test(object):
....:     Finite = imported_lazy_class_attribute('sage.categories.finite_permutation_groups', 'FinitePermutationGroups')
....:     
sage: Test.Finite
<class 'sage.categories.finite_permutation_groups.FinitePermutationGroups'>

Alternatively, since LazyImport has some __get__: We could make the lazy import behave as a class attribute, if it is used as attribute of a class (__get__ knows whether it is used as attribute of a class.

comment:2 Changed 5 years ago by SimonKing

Strange. Shouldn't LazyImport already act as a class attribute? After all, I see the following code:

            if owner is None:
                if self._namespace and self._namespace[alias] is self:
                    self._namespace[alias] = self._object
            else:
                from inspect import getmro
                for cls in getmro(owner):
                    if cls.__dict__.get(alias, None) is self:
                        setattr(cls, alias, self._object)
                        break

This should actually do the trick.

comment:3 Changed 5 years ago by SimonKing

In the example of the ticket description, I suppose you simply need to proved as_name="Associative", and then it should work. Can't test it right now, though.

comment:4 follow-up: Changed 5 years ago by SimonKing

The only (mild) problems I see:

  1. Why is the _get_object, if it does what ostensibly is the job of __get__?
  2. Why is the imported object put into the dict of a class, but not into the dict of an instance? _get_object can't do it, since it doesn't know the instance.

Hence, I am all for removing _get_object.

comment:5 in reply to: ↑ 4 Changed 5 years ago by SimonKing

  • Status changed from new to needs_info
  • Work issues set to Do we need a lazy import to act as lazy instance attribute?
sage: from sage.misc.lazy_import import LazyImport
sage: class A:
....:     Associative = LazyImport('sage.categories.magmas', 'Magmas', 'Associative')
....:     
sage: A.Associative
<class 'sage.categories.magmas.Magmas'>
sage: A.__dict__['Associative']
<class 'sage.categories.magmas.Magmas'>

Hence, unless you agree that we should change the lazy import statement so that it additionally acts as a lazy instance attribute, then this ticket is invalid.

comment:6 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:7 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:9 Changed 2 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Dependencies set to #22752
  • Milestone changed from sage-6.4 to sage-8.0
  • Type changed from PLEASE CHANGE to defect
  • Work issues Do we need a lazy import to act as lazy instance attribute? deleted

comment:10 Changed 2 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/15648

comment:11 Changed 2 years ago by jdemeyer

  • Commit set to d5f23484bd61b280dcf5b716f58fe31a6847e18d
  • Status changed from needs_info to needs_review

New commits:

94302caDo not declare functions/methods as "cdef inline"
8aac18aVarious improvements to lazy imports
8ec9f5dLazy import context based on __import__
d50f9a6Make "with lazyimport" context more thread-safe
3d62c9eUpdate documentation
d5f2348Search for object in __get__

comment:12 Changed 2 years ago by git

  • Commit changed from d5f23484bd61b280dcf5b716f58fe31a6847e18d to 796a45277a23c14ea86126f5e08349f2c25c34d2

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

9e1208blazyimport -> _lazyimport_
796a452Search for object in __get__

comment:13 Changed 2 years ago by jdemeyer

  • Milestone changed from sage-8.0 to sage-duplicate/invalid/wontfix
  • Resolution set to wontfix
  • Status changed from needs_review to closed

comment:14 Changed 2 years ago by jdemeyer

  • Authors Jeroen Demeyer deleted
  • Branch u/jdemeyer/ticket/15648 deleted
  • Commit 796a45277a23c14ea86126f5e08349f2c25c34d2 deleted
  • Resolution wontfix deleted
  • Status changed from closed to new

comment:15 Changed 2 years ago by jdemeyer

  • Dependencies #22752 deleted
Note: See TracTickets for help on using tickets.