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

Refactor distributions sagemath-{objects,categories} through sagemath-{environment,repl} #33812

Closed
mkoeppe opened this issue May 5, 2022 · 114 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented May 5, 2022

Currently sagemath-categories distributes a superset of sagemath-objects, and both distributions include a copy of the doctester for their doctesting. We remove this duplication by declaring install-requires and extras-require.

The dependencies are illustrated in the updated developer's guide - https://e019add9a8540b8213dfc563e8e4bda04777282c--sagemath-tobias.netlify.app/developer/packaging_sage_library.html#hierarchy-of-distribution-packages

As #33817 is a dependency, this ticket can be tested by running

./bootstrap && make SAGE_CHECK=yes pypi-wheels

This test is also run as part of the "Build&Test" workflow.

CC: @kwankyu

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: e5d1070

Reviewer: Kwankyu Lee

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone May 5, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 6, 2022

Changed dependencies from #29941, #28925 to #29941, #28925, #33817

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Commit: f6a2ee2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

0b4d1fdpkgs/sagemath-objects: Disable use of the Sage doctester
bbd9f05build/pkgs/sagemath_objects/spkg-install: tox --force-dep does not work, remove
f6a2ee2pkgs/sagemath-*/tox.ini: Add sagepython-norequirements-sagewheels-nopypi

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

comment:5

Now that we are testing sagemath-objects with the stuff now in sagemath-environment stripped away, dependencies become apparent that need to be removed.

[sagemath_objects-9.6.rc3]   File "sage/misc/lazy_import.pyx", line 69, in init sage.misc.lazy_import (build/cythonized/sage/misc/lazy_import.c:10965)
[sagemath_objects-9.6.rc3]     from . import sageinspect
[sagemath_objects-9.6.rc3]   File "/Users/mkoeppe/s/sage/sage-rebasing/pkgs/sagemath-objects/.tox/sagepython-norequirements-sagewheels-nopypi/lib/python3.10/site-packages/sage/misc/sageinspect.py", line 122, in <module>
[sagemath_objects-9.6.rc3]     from sage.env import SAGE_LIB
[sagemath_objects-9.6.rc3] ModuleNotFoundError: No module named 'sage.env'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Refactor distributions sage-setup, sagemath-{objects,categories} through sagemath-{environment,repl} Refactor distributions sagemath-{objects,categories} through sagemath-{environment,repl} May 8, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

comment:7

So this needs #33037 (Remove use of SAGE_LIB)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Changed dependencies from #29941, #28925, #33817 to #29941, #28925, #33817, #33821

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Changed dependencies from #29941, #28925, #33817, #33821 to #29941, #28925, #33817, #33821, #33822

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

eef9e41build/pkgs/python3/spkg-configure.m4: Check for module ensurepip
1340d95build/pkgs/python3/distros/debian.txt: Add python3-venv
823bfd2Merge #33822
ffed245build/pkgs/sagemath_objects/spkg-install: Remove workaround for missing ensurepip

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from f6a2ee2 to ffed245

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from ffed245 to e3be906

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

e3be906.github/workflows/build.yml: Install python3-venv

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

5f311f5build/pkgs/sagemath_*/dependencies: Fix SAGE_CHECK dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from e3be906 to 5f311f5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

c264e7fbuild/make/Makefile.in: Handle SAGE_CHECK... for script packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from 5f311f5 to c264e7f

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 8, 2022

Work Issues: Overriding SAGE_EDITABLE does not work

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

de18ba0build/bin/sage-build-env[-config]: Do not override SAGE_EDITABLE if it is set already

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from c264e7f to de18ba0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

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

202485fTrac #33793: replace SAGE_TMP in SPYX_TMP.
827c527Trac #33793: move spyx_tmp() to sage.misc.temporary_file.
fc25b79Trac #33793: cache spyx_tmp() by hand.
3068676Merge #33793
b41da0csrc/sage/misc/sageinspect.py: Remove use of SAGE_LIB
877fe16src/sage/misc/cachefunc.pyx: Remove use of SAGE_LIB
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
4f5f1f3Merge #33821

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2022

Changed commit from de18ba0 to 4f5f1f3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

Changed commit from 1fe31c8 to 9fd5817

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2022

Changed dependencies from #33817 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2022

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

e41e277Merge tag '9.7' into t/33812/refactor_distributions_sage_setup__sagemath__objects_categories__through_sagemath__environment_repl_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2022

Changed commit from 9fd5817 to e41e277

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

comment:60

Questions:

  1. In dependencies files, sagemath-categories depends on cysignals gmpy2 sagemath_objects and sagemath-objects depends on cysignals gmpy2. Does cysignals gmpy2 need to be repeated?

  2. In the hierarchy diagram, sagemath-repl depends on both sagemath-environment and sagemath-objects, but in the dependencies file of sagemath-repl spkg, I see sagemath_objects ipython. Why sagemath-environment is missing?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 25, 2022

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

996461abuild/pkgs/sagemath_repl/dependencies: Update from pkgs/sagemath-repl/setup.cfg.m4
0ffdd57pkgs/sagemath-repl/setup.cfg.m4: Update python-requires
141318fbuild/pkgs/sagemath_categories/dependencies: Remove dependencies implied by sagemath_objects dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 25, 2022

Changed commit from e41e277 to 141318f

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

comment:62

Replying to Kwankyu Lee:

  1. In the hierarchy diagram, sagemath-repl depends on both sagemath-environment and sagemath-objects, but in the dependencies file of sagemath-repl spkg, I see sagemath_objects ipython. Why sagemath-environment is missing?

Sorry.. The hierarchy diagram is the hierarchy of the distribution pkgs. I now see the correct dependencies in install_requires.


New commits:

996461abuild/pkgs/sagemath_repl/dependencies: Update from pkgs/sagemath-repl/setup.cfg.m4
0ffdd57pkgs/sagemath-repl/setup.cfg.m4: Update python-requires
141318fbuild/pkgs/sagemath_categories/dependencies: Remove dependencies implied by sagemath_objects dependencies

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 25, 2022

comment:63

The distribution packages declare runtime dependencies (setup.cfg install_requires) and also build-time dependencies (pyproject.toml). I think in the diagram I only show the (covers of) runtime dependencies.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

comment:64

Then perhaps the change of the commit 996461a is not strictly necessary? (if it is practically no-op and does not create any overhead, then no problem).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 25, 2022

comment:65

No, you spotted it correctly in comment:62. sagemath_environment was missing, and 996461a corrected that. (Also ipywidgets was missing.)

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

Changed commit from 141318f to 3da1811

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

Changed reviewer from https://github.com/mkoeppe/sage/runs/7511007092 to Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

New commits:

3da1811Wrapping lines of README.rst

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

comment:68

What is the logic of the changes in lazy_import.pyx? I have no idea...

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 25, 2022

comment:69

Replying to Kwankyu Lee:

What is the logic of the changes in lazy_import.pyx? I have no idea...

It seems a provision to avoid ImportError in the situation when sage.misc.lazy_import is used when sage.features package is not present...

How about adding a comment to the code to explain the situation more concretely?

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 26, 2022

Changed commit from 3da1811 to e5d1070

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 26, 2022

comment:71

Thanks for the comment.

9.7b6 below is 9.7 in other files, though I think it doesn't matter as explained in the comment.

+++ b/build/pkgs/sagemath_repl/install-requires.txt
@@ -0,0 +1,2 @@
+# This file is updated on every release by the sage-update-version script
+sagemath-repl ~= 9.7b6

I tested this ticket in several ways. It works well. It looks good to me.


New commits:

e5d1070src/sage/misc/lazy_import.pyx: Add comments

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 26, 2022

comment:72

Thank you!

@vbraun
Copy link
Member

vbraun commented Sep 28, 2022

@vbraun vbraun closed this as completed in 024b81f Sep 28, 2022
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 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

3 participants