Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring tool to fix modularization anti-patterns #34945

Closed
mkoeppe opened this issue Jan 28, 2023 · 40 comments
Closed

Refactoring tool to fix modularization anti-patterns #34945

mkoeppe opened this issue Jan 28, 2023 · 40 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2023

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

@mkoeppe mkoeppe added this to the sage-9.8 milestone Jan 28, 2023
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 28, 2023

Branch: u/mkoeppe/replace_dot_all

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2023

Commit: 294e3e9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2023

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

7a54612New command 'sage --fiximports', invokes sage.misc.replace_dot_all
294e3e9src/sage/misc/replace_dot_all.py: Break some long lines

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2023

Changed commit from 294e3e9 to b8b4372

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2023

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

057dfb5src/sage/misc/replace_dot_all.py: Pass package_regex, not regex including 'import '
b8b4372src/sage/misc/replace_dot_all.py (make_replacements_in_file): Add optional arg 'output'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from b8b4372 to 01efec0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

46a15bfsrc/sage/misc/replace_dot_all.py: Pass package_regex, not regex including 'import ' (fixup)
2cbbf2dsrc/sage/misc/replace_dot_all.py: Use more 'with open'
29e5ddbsrc/sage/misc/replace_dot_all.py: Replace sort_log_messages by print_log_messages
01efec0src/sage/misc/replace_dot_all.py: Use endswith()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 01efec0 to 810d982

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 810d982 to dbcccc1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from dbcccc1 to 491805d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 491805d to bf5e6d2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

8cef447src/sage/misc/replace_dot_all.py: Restore print_log_messages lost in previous commit
aa5f1e4src/sage/misc/replace_dot_all.py: Clean docstrings, uncamelcaps
202cdc5src/sage/misc/replace_dot_all.py: Update doctests
bf5e6d2src/sage/misc/replace_dot_all.py: Simplify some 'if' conditions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from bf5e6d2 to b426778

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

b426778src/sage/misc/replace_dot_all.py: Doc updates, pycodestyle fixes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from b426778 to 685680b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 685680b to 0b8dd2f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

0b8dd2fsrc/sage/misc/replace_dot_all.py: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 0b8dd2f to 1c19d43

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

d296b7csrc/sage/misc/replace_dot_all.py: Replace 'row number', 'line number' by :
ac83a49src/sage/misc/replace_dot_all.py: Use __import__
fe617e9src/sage/misc/replace_dot_all.py: Fold parse_arguments into main
1c19d43src/sage/misc/replace_dot_all.py: Fix doctests

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

Reviewer: Matthias Koeppe, ...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

Changed author from Alex Chandler to Alex Chandler, Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

23dff24src/sage/misc/replace_dot_all.py: Simplify print_log_messages, fold into main

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 1c19d43 to 23dff24

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 23dff24 to 568c538

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

568c538src/sage/misc/replace_dot_all.py: Use os.path.join more

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

eef9cd5src/doc/en/developer/packaging_sage_library.rst: Mention relint, sage --fiximports

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from 568c538 to eef9cd5

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

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"

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

Changed commit from eef9cd5 to e74dbf6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2023

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

e74dbf6src/sage/misc/replace_dot_all.py: Add copyright notice

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

Dependencies: #34849

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

comment:20

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

@alexchandler100
Copy link
Contributor

comment:21

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

@alexchandler100
Copy link
Contributor

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

@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 29, 2023
vbraun pushed a commit that referenced this issue Feb 12, 2023
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
vbraun pushed a commit that referenced this issue Feb 12, 2023
….*.all for namespace packages

Using `./sage -fiximports` from #34945.

Also remove trailing whitespace in the affected files.

Part of Meta-ticket #32414

URL: https://trac.sagemath.org/34947
Reported by: mkoeppe
Ticket author(s): Alex Chandler
Reviewer(s): Travis Scrimshaw, Matthias Koeppe
vbraun pushed a commit that referenced this issue Feb 12, 2023
…ce packages

Using `./sage -fiximports` from #34945.

Also remove trailing whitespace in the affected files.

Part of Meta-ticket #32414

URL: https://trac.sagemath.org/34952
Reported by: mkoeppe
Ticket author(s): Alex Chandler
Reviewer(s): David Coudert
vbraun pushed a commit that referenced this issue Feb 12, 2023
…l for namespace packages

Using `./sage -fiximports` from #34945.

Also remove trailing whitespace in the affected files.

Part of Meta-ticket #32414

URL: https://trac.sagemath.org/34956
Reported by: mkoeppe
Ticket author(s): Alex Chandler, Matthias Koeppe
Reviewer(s): Travis Scrimshaw
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2023

Merged in 10.0.beta0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants