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

Remove imports from sage.arith.all in the Sage library #34197

Closed
mkoeppe opened this issue Jul 18, 2022 · 16 comments
Closed

Remove imports from sage.arith.all in the Sage library #34197

mkoeppe opened this issue Jul 18, 2022 · 16 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 18, 2022

sage.arith is a namespace package after #33011, so we remove imports from sage.arith.all throughout the library

https://doc.sagemath.org/html/en/developer/packaging_sage_library.html#module-level-runtime-dependencies

Component: refactoring

Branch/Commit: u/mkoeppe/remove_imports_from_sage_arith_all_in_the_sage_library @ d6ce90c

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone Jul 18, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 18, 2022

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2022

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

1011578Remove some imports from sage.arith.all

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2022

Commit: 1011578

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 18, 2022

Author: Matthias Koeppe, ...

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 26, 2022

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

e044d0dgit grep -l 'sage.arith.all import' | xargs sed -E -i.bak 's/sage.arith.all import (lcm|LCM) *$/sage.arith.functions import \1/'
7ab066eRemove some imports from sage.arith.all

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 26, 2022

Changed commit from 1011578 to 7ab066e

@mantepse
Copy link
Contributor

comment:7

The changes in sfa.py and generating_series.py will probably create a (trivial) conflict (which requires manual interaction) with #34470.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2022

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

2c9bea2git grep -l 'sage.arith.all import' | xargs sed -E -i.bak 's/sage.arith.all import (lcm|LCM) *$/sage.arith.functions import \1/'
27d0333Remove some imports from sage.arith.all

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2022

Changed commit from 7ab066e to 27d0333

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2022

comment:9
        from . import padics
      File "/__w/sagetrac-mirror/sagetrac-mirror/src/sage/schemes/elliptic_curves/padics.py", line 26, in <module>
        from . import padic_lseries as plseries
      File "/__w/sagetrac-mirror/sagetrac-mirror/src/sage/schemes/elliptic_curves/padic_lseries.py", line 75, in <module>
        from sage.arith.functions import LCM
    ImportError: cannot import name 'LCM' from 'sage.arith.functions' (/__w/sagetrac-mirror/sagetrac-mirror/src/sage/arith/functions.cpython-38-x86_64-linux-gnu.so)

as seen in https://github.com/sagemath/sagetrac-mirror/actions/runs/3440141879/jobs/5738263597

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2022

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

d6ce90csrc/sage/schemes/elliptic_curves/padic_lseries.py: Fix up import

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2022

Changed commit from 27d0333 to d6ce90c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 12, 2022

comment:11

The remaining doctest failure

sage -t --random-seed=29164138248827613754957725053409603108 doc/de/a_tour_of_sage/index.rst  # 1 doctest failed

... is unrelated to the changes here

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

comment:12

We can close this ticket as a duplicate; the new tool sage -fiximports added in #34945 can make these changes automatically.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2023

Changed author from Matthias Koeppe, ... to none

@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
@mkoeppe mkoeppe closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2023
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