Refactoring tool to fix modularization anti-patterns

Question

Refactoring tool to fix modularization anti-patterns

Closed this issue 3 years ago · 40 comments

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

Depends on #34849

CC: @alexchandler100

Component: scripts

Author: Alex Chandler, Matthias Koeppe

Branch/Commit: u/mkoeppe/replace_dot_all @ e74dbf6

Reviewer: Matthias Koeppe, Alex Chandler

Issue created by migration from https://trac.sagemath.org/ticket/34945

Answer 1 · 2023-01-28T22:17:38.000Z

Branch: u/mkoeppe/replace_dot_all

Answer 2 · 2023-01-28T23:03:48.000Z

Commit: 294e3e9

Answer 3 · 2023-01-28T23:03:48.000Z

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

`7a54612`	`New command 'sage --fiximports', invokes sage.misc.replace_dot_all`
`294e3e9`	`src/sage/misc/replace_dot_all.py: Break some long lines`

Answer 4 · 2023-01-28T23:48:54.000Z

Changed commit from 294e3e9 to b8b4372

Answer 5 · 2023-01-28T23:48:54.000Z

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

`057dfb5`	`src/sage/misc/replace_dot_all.py: Pass package_regex, not regex including 'import '`
`b8b4372`	`src/sage/misc/replace_dot_all.py (make_replacements_in_file): Add optional arg 'output'`

Answer 6 · 2023-01-29T00:38:27.000Z

Changed commit from b8b4372 to 01efec0

Answer 7 · 2023-01-29T00:38:27.000Z

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

`46a15bf`	`src/sage/misc/replace_dot_all.py: Pass package_regex, not regex including 'import ' (fixup)`
`2cbbf2d`	`src/sage/misc/replace_dot_all.py: Use more 'with open'`
`29e5ddb`	`src/sage/misc/replace_dot_all.py: Replace sort_log_messages by print_log_messages`
`01efec0`	`src/sage/misc/replace_dot_all.py: Use endswith()`

Answer 8 · 2023-01-29T01:00:22.000Z

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

`810d982`	`src/sage/misc/replace_dot_all.py: Replace -l with positional argument, handle multiple files/dirs, do not use directory prefix`

Answer 9 · 2023-01-29T01:00:22.000Z

Changed commit from 01efec0 to 810d982

Answer 10 · 2023-01-29T01:04:48.000Z

Changed commit from 810d982 to dbcccc1

Answer 11 · 2023-01-29T01:04:48.000Z

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

`dbcccc1`	`src/sage/misc/replace_dot_all.py: Replace -l with positional argument, handle multiple files/dirs, do not use directory prefix`

Answer 12 · 2023-01-29T01:05:40.000Z

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

`491805d`	`src/sage/misc/replace_dot_all.py: Replace -l with positional argument, handle multiple files/dirs, do not use directory prefix`

Answer 13 · 2023-01-29T01:05:40.000Z

Changed commit from dbcccc1 to 491805d

Answer 14 · 2023-01-29T02:14:41.000Z

Changed commit from 491805d to bf5e6d2

Answer 15 · 2023-01-29T02:14:41.000Z

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

`8cef447`	`src/sage/misc/replace_dot_all.py: Restore print_log_messages lost in previous commit`
`aa5f1e4`	`src/sage/misc/replace_dot_all.py: Clean docstrings, uncamelcaps`
`202cdc5`	`src/sage/misc/replace_dot_all.py: Update doctests`
`bf5e6d2`	`src/sage/misc/replace_dot_all.py: Simplify some 'if' conditions`

Answer 16 · 2023-01-29T02:29:05.000Z

Changed commit from bf5e6d2 to b426778

Answer 17 · 2023-01-29T02:29:05.000Z

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

`b426778`	`src/sage/misc/replace_dot_all.py: Doc updates, pycodestyle fixes`

Answer 18 · 2023-01-29T02:31:54.000Z

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

`685680b`	`src/sage/misc/replace_dot_all.py: Use isinstance instead of type(...) == ...`

Answer 19 · 2023-01-29T02:31:54.000Z

Changed commit from b426778 to 685680b

Answer 20 · 2023-01-29T02:33:39.000Z

Changed commit from 685680b to 0b8dd2f

Answer 21 · 2023-01-29T02:33:39.000Z

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

`0b8dd2f`	`src/sage/misc/replace_dot_all.py: Fixup`

Answer 22 · 2023-01-29T03:02:27.000Z

Changed commit from 0b8dd2f to 1c19d43

Answer 23 · 2023-01-29T03:02:27.000Z

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

`d296b7c`	`src/sage/misc/replace_dot_all.py: Replace 'row number', 'line number' by :`
`ac83a49`	`src/sage/misc/replace_dot_all.py: Use __import__`
`fe617e9`	`src/sage/misc/replace_dot_all.py: Fold parse_arguments into main`
`1c19d43`	`src/sage/misc/replace_dot_all.py: Fix doctests`

Answer 24 · 2023-01-29T03:03:01.000Z

Reviewer: Matthias Koeppe, ...

Answer 25 · 2023-01-29T03:03:01.000Z

Changed author from Alex Chandler to Alex Chandler, Matthias Koeppe

Answer 26 · 2023-01-29T03:11:48.000Z

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

`23dff24`	`src/sage/misc/replace_dot_all.py: Simplify print_log_messages, fold into main`

Answer 27 · 2023-01-29T03:11:48.000Z

Changed commit from 1c19d43 to 23dff24

Answer 28 · 2023-01-29T03:14:29.000Z

Changed commit from 23dff24 to 568c538

Answer 29 · 2023-01-29T03:14:29.000Z

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

`568c538`	`src/sage/misc/replace_dot_all.py: Use os.path.join more`

Answer 30 · 2023-01-29T03:21:22.000Z

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

`eef9cd5`	`src/doc/en/developer/packaging_sage_library.rst: Mention relint, sage --fiximports`

Answer 31 · 2023-01-29T03:21:22.000Z

Changed commit from 568c538 to eef9cd5

Answer 32 · 2023-01-29T03:25:51.000Z

comment:17

Alex, your code works well. I have only made minor changes that bring the command line interface in line with similar tools, and some style changes.

It has proper doctest coverage now and passes the code style tests.

If you have a chance to look through my changes, you can add your name to "Reviewers"

Answer 33 · 2023-01-29T03:29:40.000Z

Changed commit from eef9cd5 to e74dbf6

Answer 34 · 2023-01-29T03:29:40.000Z

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

`e74dbf6`	`src/sage/misc/replace_dot_all.py: Add copyright notice`

Answer 35 · 2023-01-29T03:33:22.000Z

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
+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

Answer 36 · 2023-01-29T07:12:54.000Z

Dependencies: #34849

Answer 37 · 2023-01-29T07:12:54.000Z

comment:20

The Lint and Build&Test failures are unrelated to the changes here.

Answer 38 · 2023-01-29T17:45:09.000Z

comment:21

I've taken a look, and the changes all look great to me. I've added my name to the reviewers.

Answer 39 · 2023-01-29T17:45:09.000Z

Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, Alex Chandler

Answer 40 · 2023-02-13T00:59:56.000Z

Merged in 10.0.beta0