Opened 3 years ago

Closed 3 years ago

#22755 closed enhancement (fixed)

Various fixes to lazy imports

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.0
Component: misc Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 4dda1ae (Commits) Commit: 4dda1ae48c5cfcf4a2b0325ae80cb7d6bfb4e2dc
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Assorted fixes to lazy imports:

  1. Drop support for lazy_import(overwrite=False) which was used only in one place in a dubious way to implement a 2-level lazy import (a lazy import being lazily imported).
  1. More optimal code for binary operations like __add__ by avoiding the operator module.
  1. Move the code to replace a lazy import in a class namespace from _get_object() to __get__.
  1. Partially inline _get_object() with a new cdef inline function for the case that the object has been initialized.
  1. A lazy import without at_startup being imported at startup is now an error (it used to just print a message, which is less useful than an error traceback).
  1. Implement __matmul__ (pointless in Python 2, but useful for Python 3).
  1. Clean up the function lazy_import a bit.

Change History (23)

comment:1 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/22755

comment:2 Changed 3 years ago by jdemeyer

  • Commit set to d2bb491988d5d35323bf43858b14338023db1286
  • Status changed from new to needs_review

New commits:

94302caDo not declare functions/methods as "cdef inline"
9e5715eVarious improvements to lazy imports
d2bb491Whitespace fix

comment:3 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 3 years ago by git

  • Commit changed from d2bb491988d5d35323bf43858b14338023db1286 to 8aac18a6b165bc397ccaeaba0b596797e900b0f1

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

8aac18aVarious improvements to lazy imports

comment:6 Changed 3 years ago by git

  • Commit changed from 8aac18a6b165bc397ccaeaba0b596797e900b0f1 to f3ede0087cb94d2bce0a6edb6d99a22b1f05f8c7

Branch pushed to git repo; I updated commit sha1. New commits:

f3ede00Use exact type check in obj()

comment:7 Changed 3 years ago by jdemeyer

  • Dependencies #22753 deleted

comment:8 Changed 3 years ago by git

  • Commit changed from f3ede0087cb94d2bce0a6edb6d99a22b1f05f8c7 to 7bf2ce87733e143d979abcd79149654c594a615c

Branch pushed to git repo; I updated commit sha1. New commits:

7bf2ce8Partially inline LazyImport._get_object()

comment:9 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:10 follow-up: Changed 3 years ago by vdelecroix

You expect the following traceback to be accepted by doctests?

Traceback (most recent call last):
  File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
    self.compile_and_execute(example, compiler, test.globs)
  File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
    exec(compiled, globs)
  File "<doctest sage.misc.lazy_import.test_fake_startup[3]>", line 1, in <module>
    my_ZZ(Integer(123))
  File "sage/misc/lazy_import.pyx", line 346, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3495)
    return self._get_object()(*args, **kwds)
  File "sage/misc/lazy_import.pyx", line 210, in sage.misc.lazy_import.LazyImport._get_object (build/cythonized/sage/misc/lazy_import.c:2139)
    raise RuntimeError(f"resolving lazy import {self._name} during startup")
RuntimeError: resolving lazy import ZZ during startup

comment:11 Changed 3 years ago by git

  • Commit changed from 7bf2ce87733e143d979abcd79149654c594a615c to 4dda1ae48c5cfcf4a2b0325ae80cb7d6bfb4e2dc

Branch pushed to git repo; I updated commit sha1. New commits:

4dda1aeFix traceback

comment:12 in reply to: ↑ 10 Changed 3 years ago by jdemeyer

Replying to vdelecroix:

You expect the following traceback to be accepted by doctests?

Apparently yes (see patchbot) :-)

Anyway, I fixed it.

comment:13 follow-up: Changed 3 years ago by vdelecroix

Weird patchbot failure on quasar! Do you think it is related to your ticket?

comment:14 in reply to: ↑ 13 ; follow-up: Changed 3 years ago by jdemeyer

Replying to vdelecroix:

Weird patchbot failure on quasar! Do you think it is related to your ticket?

Almost certainly not.

comment:15 in reply to: ↑ 14 Changed 3 years ago by vdelecroix

Replying to jdemeyer:

Replying to vdelecroix:

Weird patchbot failure on quasar! Do you think it is related to your ticket?

Almost certainly not.

Note that all tests pass on some other tickets. I will relaunch it.

comment:16 Changed 3 years ago by jdemeyer

The most recent patchbot run passes all tests.

comment:17 Changed 3 years ago by vdelecroix

The documentation of the overwrite argument in lazy_import disappeard.

NOTE: though it is deprecated

Last edited 3 years ago by vdelecroix (previous) (diff)

comment:18 Changed 3 years ago by vdelecroix

Why stuff from sage.dynamics.interval_exchanges and sage.dynamics.flat_surfaces not lazily imported anymore?

comment:19 Changed 3 years ago by jdemeyer

As I tried to explain in the ticket description, it was lazily importing a lazy import.

comment:20 Changed 3 years ago by jdemeyer

Ping?

comment:21 Changed 3 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:22 Changed 3 years ago by vdelecroix

done!

comment:23 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/22755 to 4dda1ae48c5cfcf4a2b0325ae80cb7d6bfb4e2dc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.