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 use of SAGE_LIB in sage.misc #33821

Closed
mkoeppe opened this issue May 8, 2022 · 21 comments
Closed

Remove use of SAGE_LIB in sage.misc #33821

mkoeppe opened this issue May 8, 2022 · 21 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented May 8, 2022

We delay importing lazy_import_cache until a lazy star import is done (which the Sage library never does).

We change two copies of essentially the same code that tries to use SAGE_SRC and SAGE_LIB to turn a source filename to a qualified package name so that

  • it does not fail when sage.env.SAGE_SRC cannot be imported or is empty
  • it replaces the use of SAGE_LIB by using sage.__path__ (to handle namespace packages correctly)

part of Meta-ticket #33037 (Remove use of SAGE_LIB and SAGE_EXTCODE variables)

Depends on #33793

CC: @kiwifb @antonio-rojas @tornaria

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 748d9fb

Reviewer: François Bissey

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone May 8, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Dependencies: #33793

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

8cacd5esrc/sage/misc/sageinspect.py: Remove import of SAGE_LIB
c8a5bdfsrc/sage/misc/lazy_import.pyx: Import lazy_import_cache only when needed
cce5385src/sage/sets/set_from_iterator.py: Remove use of SAGE_LIB

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Commit: cce5385

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from cce5385 to b39cf31

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

b39cf31src/sage/misc/lazy_import.pyx: Update doctest

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2022

comment:6

The failure in "Build&Test" is in sage -t --random-seed=54174803604868423169944467131116599504 sage/parallel/map_reduce.py , unrelated.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

Changed commit from b39cf31 to 897e103

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

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

b004ca9src/sage/misc/cachefunc.pyx: Do not fail if sage.env cannot be imported
897e103src/sage/sets/set_from_iterator.py: Do not fail if sage.env cannot be imported

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Jun 12, 2022

comment:11

You have a repeated block

+                def directories():
+                    try:
+                        from sage.env import SAGE_SRC
+                    except ImportError:
+                        pass
+                    else:
+                        if SAGE_SRC:
+                            yield normpath(os.path.join(SAGE_SRC, 'sage'))
+                    import sage
+                    yield from sage.__path__
+
+                for directory in directories():
+                    if commonprefix([filename, directory]) == directory:
+                        filename = os.path.join('sage', relpath(filename, directory))
+                        break

If it ends up being useful in more than two files, should it be factored somewhere?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2022

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

53154b3Merge tag '9.7.beta3' into t/33821/remove_use_of_sage_lib_in_sage_misc
748d9fbsage.misc.sageinspect.sage_getfile_relative: New, use it in sage.misc.cachefunc, sage.sets.set_from_iterator.Decorator

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2022

Changed commit from 897e103 to 748d9fb

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 25, 2022

comment:13

Good suggestion, done now

@kiwifb
Copy link
Member

kiwifb commented Jun 25, 2022

comment:14

LGTM, let's ship it.

@kiwifb
Copy link
Member

kiwifb commented Jun 25, 2022

Reviewer: François Bissey

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 25, 2022

comment:15

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 3, 2022

Changed branch from u/mkoeppe/remove_use_of_sage_lib_in_sage_misc to 748d9fb

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