sage.matrix: Resolve circular imports without using __init__.py
Closed this issue · 13 comments
This is preparation for making sage.matrix a namespace package in #28925.
CC: @simon-king-jena @tscrim @tobiasdiez @videlec @kiwifb
Component: refactoring
Author: Matthias Koeppe
Branch/Commit: ea0d15a
Reviewer: François Bissey
Issue created by migration from https://trac.sagemath.org/ticket/30784
Description changed:
---
+++
@@ -1 +1,2 @@
-This is preparation for making sage.matrix a namespace package in #28925.
+This is preparation for making `sage.matrix` a namespace package in #28925.
+Author: Matthias Koeppe
New commits:
ea0d15a | sage.matrix: Resolve circular imports without using __init__.py |
What's the advantage of using global variables? I think python is also caching the imported module under the hood (https://docs.python.org/3/reference/import.html#the-module-cache).
I'm imitating existing code in Cython modules here, for example in src/sage/matrix/matrix_double_dense.pyx or src/sage/rings/complex_mpc.pyx. I have not timed it, but I would guess it has smaller overhead than the import mechanism.
Green patchbot, needs review. (One other patchbot fails with an unrelated error.)
LGTM
Reviewer: François Bissey
Thanks!
Without double-checking I would guess that the python import cache is heavily optimized. But even given the minimal performance penalty, I would suggest to using a simple import in favor of the global variable. The latter is a possible source of bugs (devs forgetting to use import before using it) and heavily complicates the work of static type checking tools. But maybe I'm also missing some other advantages of this approach (@francois).
Changed branch from u/mkoeppe/sage_matrix__resolve_circular_imports_without_using___init___py to ea0d15a