Opened 9 years ago

Closed 4 years ago

#10906 closed defect (duplicate)

lazy import can break unique representation

Reported by: nthiery Owned by: jason
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: misc Keywords: lazy import, unique representation
Cc: robertwb Merged in:
Authors: Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

A fun experiment: edit sage/categories/semigroups, and add the following lines at the beginning:

from sage.misc.lazy_import import lazy_import
lazy_import('sage.rings.rational_field', 'QQ')

Then restart sage:

    sage: sage.categories.semigroups.QQ is QQ
    False

This bit me hard, because such a lazy_import indirectly changed the base ring of a matrix to be equal to QQ but not identical, which in turn broke all the linear algebra: I was getting a matrix space over QQ whose elements were generic matrices.

Change History (8)

comment:1 Changed 9 years ago by lftabera

I am having hard time inserting lazy_imports into some libraries. They break coercions and parents.

In the example above the problem cannot really be solved. QQ and a lazy_QQ will always have different id because they are different objects, "is" is comparing the memory addresses that of course are different.

My advise: never use lazy_imports for QQ, ZZ, CC, RR, SR, all the symbolic constants, Infinity...

comment:2 Changed 9 years ago by mhansen

lazy_import should only really be used on callables, even then there are possibilities to break stuff. I'm not convinced that lazy_import is necessarily that helpful for these reasons.

comment:3 follow-up: Changed 9 years ago by robertwb

Lazy import are mostly used to avoid importing expensive modules when you might want to use their functionality. It would be both inefficient and messy to use at a fine-graned level. Whole modules are nice to lazily import, e.g.

sage: lazy_import('sage.rings', 'all', 'rings')
sage: rings
<module 'sage.rings.all' from '/mnt/usb1/scratch/robertwb/sage-4.6.2.rc1/local/lib/python2.6/site-packages/sage/rings/all.pyc'>

then you can use rings.X and it will resolve lazily. This can be especially useful for heavy, external dependencies (e.g. matplotlib).

As for the basic objects like ZZ, QQ, CC, etc. there's no reason to lazily import them, as those modules will always be already loaded. The safest are modules and callables, using lazy_import objects is just fine, passing them around is more dangerous.

comment:4 in reply to: ↑ 3 Changed 9 years ago by nthiery

Replying to robertwb:

Lazy import are mostly used to avoid importing expensive modules when you might want to use their functionality. It would be both inefficient and messy to use at a fine-graned level. Whole modules are nice to lazily import. As for the basic objects like ZZ, QQ, CC, etc. there's no reason to lazily import them, as those modules will always be already loaded.

Well, except for category code, especially in the basic categories! And that's precisely where lazy importation is a nice idiom, since this code is loaded very early and one does not want to cause loops there. But lazy importing the appropriate modules instead will indeed work too. Thanks for the tip.

Now, to avoid getting confused in case of misuse (which can lead to very tricky situations to debug), what about changing the repr for lazy imported object so that one would get something like:

    sage: lazy_import('sage.all', 'ZZ', 'my_ZZ')
    sage: def bla(x = my_ZZ):
    ...       return x
    sage: bla()
    Lazy import of Integer Ring
    sage: bla() is ZZ
    False
    sage: bla()(1)
    1
    sage: bla() is ZZ
    False

comment:5 Changed 9 years ago by robertwb

Good point about categories. As for printing them out like that would make them very unfriendly for top-level use. I could see this being a useful flag to set for debugging though (and for a slew of other objects, so we'd have "Gap(1)" and "Pari(1)" and "Maxima(1)" instead of just "1" for all of them.

For our sanity, I could see rejecting lazy import objects for category bases. It may also make sense to not delegate equality (and hashcode) operations.

comment:6 Changed 4 years ago by jdemeyer

  • Milestone set to sage-duplicate/invalid/wontfix
  • Reviewers set to Jeroen Demeyer
  • Status changed from new to needs_review

Duplicate of #19628 (which has a lot more discussion).

comment:7 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:8 Changed 4 years ago by vbraun

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.