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

sage.misc.latex, sage.repl.display.fancy_repr: Make imports more local #32634

Closed
mkoeppe opened this issue Oct 5, 2021 · 10 comments
Closed

sage.misc.latex, sage.repl.display.fancy_repr: Make imports more local #32634

mkoeppe opened this issue Oct 5, 2021 · 10 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 5, 2021

(cherry-picked from #32432)

CC: @mwageringel @kwankyu

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: bdd0325

Reviewer: Kwankyu Lee

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 5, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2021

Commit: bdd0325

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2021

New commits:

22f125bsrc/sage/misc/latex.py: Move import from sage.misc.sage_eval into method
0809840src/sage/misc/latex.py: Move import of have_program into methods
bdd0325sage.repl.display.fancy_repr: For isinstance testing, import abc Matrix from sage.structure.element

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 6, 2021

comment:4

What is the rationale of moving module-level import of have_program to function level?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2021

comment:5

Most modules that import from sage.misc.latex only import the latex function, which does not actually need to run any program; so removing this module-level import removes the indirect dependency on sage.misc.ostools (and hence on CPython- and Unix-specific code).

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 6, 2021

comment:6

Replying to @mkoeppe:

Most modules that import from sage.misc.latex only import the latex function, which does not actually need to run any program; so removing this module-level import removes the indirect dependency on sage.misc.ostools (and hence on CPython- and Unix-specific code).

If we need to care of these dependency issues because of the modularization effort, it might be useful to have a patchbot plugin to watch out increase of dependency among sage modules.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 6, 2021

Reviewer: Kwankyu Lee

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2021

comment:7

Thanks for the review.

Yes, testing infrastructure for this is in the works - see #32432

@vbraun
Copy link
Member

vbraun commented Oct 19, 2021

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

3 participants