Remove src/sage/__init__.py
Closed this issue · 39 comments
Follow-up from #33011.
Because of the complication described in #33011, we not only remove the file from the source tree but also remove it from site-packages before building sagelib.
Follow-up:
- make sagemath-standard, sage-setup depend on sagemath-environment
Depends on #33011
Component: refactoring
Author: Matthias Koeppe
Branch: a77b971
Reviewer: Kwankyu Lee
Issue created by migration from https://trac.sagemath.org/ticket/34187
If I remove __init__.py from src/sage and pkgs/sagemath-standard/build/lib.macosx-12-x86_64-cpython-39/sage, then sage is built without problem, but doctest fails. Is this expected?
I'm getting
sage -t --random-seed=29572225487268684535188462271558972615 src/sage/misc/sageinspect.py # 1 doctest failed
sage -t --random-seed=29572225487268684535188462271558972615 src/sage_setup/find.py # 13 doctests failed
sage -t --random-seed=29572225487268684535188462271558972615 src/sage_setup/clean.py # 4 doctests failed
sage -t --random-seed=29572225487268684535188462271558972615 src/sage/misc/edit_module.py # 1 doctest failed
Last 10 new commits:
1326e17 | src/sage/numerical/__init__.py: Remove |
5aebcf3 | src/sage_docbuild/builders.py: Handle namespace packages |
bba5284 | src/sage_setup/find.py: Update doctest output |
56e9123 | Merge #28925 |
6a76779 | Merge #28925 |
bd185b5 | Merge tag '9.7.beta5' into t/33011/remove___init___py_files_for_packages_designated_to_be_namespace_packages |
39aa2f1 | src/sage_setup/command/sage_install.py (sage_clean): Clean __init__.py when installed package was ordinary, source package is namespace |
d3bdb46 | Merge #33011 |
7336e9d | build/pkgs/sagelib/spkg-install: Remove installed sage/__init__.py |
f447a11 | src/sage/__init__.py: Remove |
> /Users/mkoeppe/s/sage/sage-rebasing/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage_setup/find.py(278)_cythonized_dir()
276 src_dir = Path(src_dir)
277 sage = import_module('sage')
--> 278 d = Path(sage.__file__).resolve().parent.parent
279 editable_install = d == src_dir.resolve()
280 if editable_install:
this use of sage.__file__ now needs to be changed
Branch pushed to git repo; I updated commit sha1. New commits:
01f9143 | src/sage_setup: Fix for namespace package sage |
Branch pushed to git repo; I updated commit sha1. New commits:
9627219 | src/sage/misc/edit_module.py: Update doctest |
Branch pushed to git repo; I updated commit sha1. New commits:
e884c9e | src/sage/misc/sageinspect.py: Update doctest |
Author: Matthias Koeppe
Perhaps we have to respect the deprecation period of isfunction() defined in sage and deprecated by #32479. It ends after two months from now.
Branch pushed to git repo; I updated commit sha1. New commits:
7a0ff3f | src/sage/all.py: Restore sage.isfunction as a LazyImport |
Sorry for pointing to a minor thing, but the deprecation message is somewhat misleading as the function in sage.misc.sageinspect is is_function_or_cython_function.
sage: isfunction(f)
<ipython-input-3-631f9d828a9a>:1: DeprecationWarning:
Importing isfunction from here is deprecated. If you need to use it, please import
it directly from sage.misc.sageinspect
See https://trac.sagemath.org/32479 for details.
isfunction(f)
True
sage: from sage.misc.sageinspect import isfunction
---------------------------------------------------------------------------
ImportError Traceback (most recent call last)
<ipython-input-4-53c0fed64cdc> in <module>
----> 1 from sage.misc.sageinspect import isfunction
ImportError: cannot import name 'isfunction' from 'sage.misc.sageinspect' (/Users/kwankyu/GitHub/sage-temp/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/misc/sageinspect.py)
Perhaps moving isfunction in sage.__init__.py to sage.all is a solution?
Branch pushed to git repo; I updated commit sha1. New commits:
dc2ddb5 | src/sage/misc/lazy_import.pyx: Improve deprecation message when as_ is in use |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
adb89ff | src/sage/misc/lazy_import.pyx: Improve deprecation message when as_ is in use |
I've implemented a more general solution:
sage: from sage import isfunction
sage: isfunction
/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.8/lib/python3.8/site-packages/sage/repl/display/fancy_repr.py:272: DeprecationWarning:
Importing isfunction from here is deprecated; please use "from sage.misc.sageinspect import is_function_or_cython_function as isfunction" instead.
See https://trac.sagemath.org/32479 for details.
output = repr(obj)
<function is_function_or_cython_function at 0x10f957ee0>
That is a right solution. Thanks.
As a side remark, it seems to me that the lazy_import with deprecation capability is misnamed since "lazy import" itself has nothing to do with deprecation. Perhaps LazyImport object can just have a hook to a function, which is called when import happens, and we may have deprecated_import that issues the deprecation warning when import happens. Thus we have deprecated_import separate from the plain lazy_import.
I don't know, I think it's just an extra feature of lazy_import that is activated by a keyword. I don't see anything wrong with that
Replying to @mkoeppe:
I don't know, I think it's just an extra feature of
lazy_importthat is activated by a keyword. I don't see anything wrong with that.
a lazy_import with deprecation keyword is to be removed after the deprecation period while a plain lazy_import is to remain. What seems wrong to me is that this distinction is implicitly made by the mere presence of a keyword argument.
Anyway, this ticket looks good to me.
doctest failures due to the changed deprecation message:
sage -t src/sage/functions/other.py # 1 doctest failed
sage -t src/sage/tests/books/computational-mathematics-with-sagemath/graphique_doctest.py # 1 doctest failed
sage -t src/sage/modular/modform/find_generators.py # 2 doctests failed
sage -t src/sage/groups/matrix_gps/homset.py # 1 doctest failed
sage -t src/sage/schemes/hyperelliptic_curves/all.py # 4 doctests failed
sage -t src/sage/rings/all.py # 3 doctests failed
Ah, I forgot about that
Branch pushed to git repo; I updated commit sha1. New commits:
a77b971 | Update doctest output for changed lazy_import deprecation warnings |
Reviewer: Kwankyu Lee
LGTM.
Thank you!
Changed branch from u/mkoeppe/remove_src_sage___init___py to a77b971
When building surface-dynamics (a Cython project that builds against sagelib) we are getting
surface_dynamics/flat_surfaces/origamis/origami_dense.pyx:39:8: 'sage/structure/element.pxd' not found
It seems to me that Cython does not find the .pxd file anymore because the __init__.py in the base directory is missing now.
This seems to be cython/cython#2918 essentially.
We could patch the cython packaged in conda-forge with cython/cython#4918 but maybe there is some way to fix this that works for unpatched Cythons?