Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nthiery opened this issue Jan 8, 2014 · 18 comments
Open

Comments

@nthiery
Copy link
Contributor

nthiery commented Jan 8, 2014

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'>

Component: misc

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

@nthiery nthiery added this to the sage-6.1 milestone Jan 8, 2014
@simon-king-jena
Copy link
Member

comment:1

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.

@simon-king-jena
Copy link
Member

comment:2

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.

@simon-king-jena
Copy link
Member

comment:3

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.

@simon-king-jena
Copy link
Member

comment:4

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.

@simon-king-jena
Copy link
Member

Work Issues: Do we need a lazy import to act as lazy instance attribute?

@simon-king-jena
Copy link
Member

comment:5
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.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Author: Jeroen Demeyer

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Dependencies: #22752

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Changed work issues from Do we need a lazy import to act as lazy instance attribute? to none

@jdemeyer jdemeyer added the t: bug label Apr 6, 2017
@jdemeyer jdemeyer modified the milestones: sage-6.4, sage-8.0 Apr 6, 2017
@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Branch: u/jdemeyer/ticket/15648

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Commit: d5f2348

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

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__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2017

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

9e1208blazyimport -> _lazyimport_
796a452Search for object in __get__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 6, 2017

Changed commit from d5f2348 to 796a452

@jdemeyer jdemeyer removed this from the sage-8.0 milestone Apr 6, 2017
@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Changed commit from 796a452 to none

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Changed author from Jeroen Demeyer to none

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Changed branch from u/jdemeyer/ticket/15648 to none

@jdemeyer jdemeyer reopened this Apr 6, 2017
@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 6, 2017

Changed dependencies from #22752 to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants