sage.categories: Remove module-level imports of sage.rings, sage.algebras, sage.matrix, sage.misc.latex, etc.
Closed this issue · 25 comments
This is for a cleaner, filtered structure of our Python modules, at least regarding their load time. This is preparation for #29865 (sage-objects).
The idea is that sage.categories should be more abstract than any of the implemented algebraic structures in sage.rings, sage.algebras, sage.modules, ... Therefore it should only have a (module load time) dependence on sage.structure and sage.misc.
The present ticket changes module-level imports to method-level imports so that they are resolved later than at module load time.
At run time, of course, there are additional dependencies. For example, as noted in #29705, the output of .cardinality() must be an integer (ZZ).
Also, obviously the doctests need a larger part of the library.
(split out from #29865)
Depends on #29869
Depends on #16351
CC: @videlec @tscrim @orlitzky @nthiery @timokau @fchapoton
Component: refactoring
Author: Matthias Koeppe
Branch/Commit: 27f2dab
Reviewer: Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/29873
New commits:
907feeb | Move attrcall and friends from sage.misc.misc to new module sage.misc.call |
a5453bf | Fixup: Add src/sage/misc/call.py |
64c5701 | lazy_import from sage.misc.call with deprecation |
65414f7 | Fix imports and one deprecation warning |
b9314d4 | sage.misc.call: Add standard header information, add to reference manual |
5e27414 | sage.categories.crystals: Make import of sage.misc.latex local to a method |
6e0fa7c | sage.categories: Make imports from sage.rings, .sets, .combinat, .plot, .matrix local to methods |
Description changed:
---
+++
@@ -2,9 +2,9 @@
The idea is that `sage.categories` should be more abstract than any of the implemented algebraic structures in `sage.rings`, `sage.algebras`, `sage.modules`, ... Therefore it should only have a (module load time) dependence on `sage.structure` and `sage.misc`.
+The present ticket changes module-level imports to method-level imports so that they are resolved later than at module load time.
+
At run time, of course, there are additional dependencies. For example, as noted in #29705, the output of `.cardinality()` must be an integer (ZZ).
-
-The present ticket changes module-level imports to method-level imports so that they are resolved later than at module load time.
Also, obviously the doctests need a larger part of the library.
Reviewer: Travis Scrimshaw
In this code, these extra imports at the method level will not have a meaningful effect on the run time, so I am giving this a positive review based on that. However, this is something to be aware of as there can be functions in the category framework that are used in tighter loops.
Thanks! Yes, let's watch out for performance impacts of some of the upcoming changes.
Actually now I see
File "/local/sage-patchbot/sage/local/lib/python3.7/site-packages/sage/combinat/backtrack.py", line 76, in <module>
from sage.sets.recursively_enumerated_set import RecursivelyEnumeratedSet_generic
File "sage/sets/recursively_enumerated_set.pyx", line 179, in init sage.sets.recursively_enumerated_set (build/cythonized/sage/sets/recursively_enumerated_set.c:12654)
ImportError: cannot import name SearchForest
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6024ffd | src/sage/misc/call.py: register_unpickle_override for call_method |
f3afd30 | sage.categories.crystals: Make import of sage.misc.latex local to a method |
6433a56 | sage.categories: Make imports from sage.rings, .sets, .combinat, .plot, .matrix local to methods |
cyclic import, it seems
sage.combinat.backtrack notes a relevant todo:
- For now the code of :class:`SearchForest` is still in
``sage/combinat/backtrack.py``. It should be moved in
``sage/sets/recursively_enumerated_set.pyx`` into a class named
:class:`RecursivelyEnumeratedSet_forest` in a later ticket.
Branch pushed to git repo; I updated commit sha1. New commits:
364d7b3 | Move SearchForest code to sage/sets/recursively_enumerated_set.pyx |
68f7bd5 | sage.combinat.integer_vectors_mod_permgroup: Update to use RecursivelyEnumeratedSet_forest |
1764ca4 | Update remaining uses of SearchForest to use RecursivelyEnumeratedSet_forest |
1ac0a09 | sage.combinat.backtrack: Remove deprecated class SearchForest |
908acba | sage.combinat.backtrack: Remove deprecated classes TransitiveIdeal and TransitiveIdealGraded |
5c20de7 | Merge branch 't/16351/move_searchforest_code_to_sage_sets_recursively_enumerated_set_pyx' into t/29873/sage_categories_remove_module_level_imports |
One last little trivial thing, please add
# -*- coding: utf-8 -*-
as the first line to call.py. Once done, you can set a positive review.
Branch pushed to git repo; I updated commit sha1. New commits:
27f2dab | src/sage/misc/call.py: Add coding directive |
Thanks!
Changed branch from u/mkoeppe/sage_categories_remove_module_level_imports to 27f2dab