Skip to content

Commit

Permalink
Trac #34945: Refactoring tool to fix modularization anti-patterns
Browse files Browse the repository at this point in the history
The new command `sage --fiximports` replaces imports from
`sage.PAC.KAGE.all` by more specific imports when `sage.PAC.KAGE` is an
implicit namespace package.

Part of Meta-ticket #34201

URL: https://trac.sagemath.org/34945
Reported by: mkoeppe
Ticket author(s): Alex Chandler, Matthias Koeppe
Reviewer(s): Matthias Koeppe, Alex Chandler
  • Loading branch information
Release Manager committed Feb 11, 2023
2 parents c898228 + e74dbf6 commit 2cd6166
Show file tree
Hide file tree
Showing 4 changed files with 483 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/.relint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@
- name: 'namespace_pkg_all_import: import from .all of a namespace package'
hint: |
Sage library code should not import from sage.PAC.KAGE.all when sage.PAC.KAGE is an implicit
Hint: namespace package. Type import_statements("SOME_IDENTIFIER") to find a more specific import.
Hint: namespace package. Type import_statements("SOME_IDENTIFIER") to find a more specific import,
Hint: or use 'sage --fiximports' to fix automatically in the source file.
# Keep in sync with SAGE_ROOT/src/sage/misc/replace_dot_all.py
pattern: 'from\s+sage(|[.](arith|categories|combinat|ext|graphs(|[.]decompositions)|interfaces|libs|matrix|misc|numerical(|[.]backends)|rings|sets))[.]all\s+import'
filePattern: '.*[.](py|pyx|pxi)$'
error: false # Make this a warning instead of an error for now
12 changes: 12 additions & 0 deletions src/bin/sage
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,13 @@ usage_advanced() {
echo " Sage documentation for \"string\"."
echo " --search_src ... -- same as --grep"
echo " --search_doc ... -- same as --grepdoc"
echo " --fixdoctests file.py"
echo " -- Run doctests and replace output of failing doctests"
echo " with actual output."
echo " --fiximports <files|dir>"
echo " -- Replace imports from sage.PAC.KAGE.all by specific"
echo " imports when sage.PAC.KAGE is an implicit namespace"
echo " package"
fi
echo " --sh [...] -- run a shell with Sage environment variables"
echo " as they are set in the runtime of Sage"
Expand Down Expand Up @@ -974,6 +981,11 @@ if [ "$1" = '-startuptime' -o "$1" = '--startuptime' ]; then
exec sage-startuptime.py "$@"
fi

if [ "$1" = '-fiximports' -o "$1" = '--fiximports' ]; then
shift
exec sage-python -m sage.misc.replace_dot_all "$@"
fi

if [ "$1" = '-tox' -o "$1" = '--tox' ]; then
shift
if [ -n "$SAGE_SRC" -a -f "$SAGE_SRC/tox.ini" ]; then
Expand Down
4 changes: 4 additions & 0 deletions src/doc/en/developer/packaging_sage_library.rst
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ distribution -- which then must be declared as a run-time dependency.
the ``sage:`` prompt. In particular, no Sage library code should import from
:mod:`sage.rings.all`.

To audit the Sage library for such imports, use ``sage --tox -e relint``.
In most cases, the imports can be fixed automatically using the
tool ``sage --fiximports``.

- Replace module-level imports by method-level imports. Note that
this comes with a small runtime overhead, which can become
noticeable if the method is called in tight inner loops.
Expand Down
Loading

0 comments on commit 2cd6166

Please sign in to comment.