#31377 closed enhancement (fixed)
./configure enableeditable
Reported by:  mkoeppe  Owned by:  

Priority:  critical  Milestone:  sage9.3 
Component:  build  Keywords:  
Cc:  ghtobiasdiez, jhpalmieri, chapoton, vdelecroix, isuruf, ghkliem, tscrim  Merged in:  
Authors:  Tobias Diez, Matthias Koeppe  Reviewers:  Matthias Koeppe, Tobias Diez, Jonathan Kliem 
Report Upstream:  N/A  Work issues:  
Branch:  8b3f390 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #30770, #30912, #31357, #31365, #31389, #31390  Stopgaps: 
Description (last modified by )
This configure switch will install sagelib in "develop" ("editable", "inplace") mode instead of using sagelib's custom incremental build system.
This will clutter the source directory with build artifacts (which are .gitignore
'd, of course) but it has the benefit that for changes to Python files, one does not need to run ./sage b
; restarting Sage is enough.
It may also have benefits in certain develop environments that get confused by sagelib's nonstandard build system.
This ticket is based on a subset of the changes in #30371, which developed a version of setup.py
suitable for the inplace build. This version is src/setup.py
and distinct from build/pkgs/sagelib/src/setup.py
.
The configure switch switches to this version of the build system.
To test:
./bootstrap ./configure enableeditable make build
Then use and test as normal. Verify that local/lib/...sitepackages/
no longer contains a copy of sage
 instead there is an "egglink" back to the source directory.
Switching to a standard build system (getting rid of the sagespecific "installation cleaner") will also simplify the next step of the modularization effort (#29705).
Change History (130)
comment:1 Changed 10 months ago by
 Dependencies set to #30770, #30912, #31357, #31362
comment:2 Changed 10 months ago by
 Dependencies changed from #30770, #30912, #31357, #31362 to #30770, #30912, #31357
comment:3 Changed 10 months ago by
 Branch set to u/mkoeppe/__configure___enable_editable
comment:4 Changed 10 months ago by
 Commit set to 3dcfbe90735023422e4f6a79594b0034290adf96
comment:5 Changed 10 months ago by
 Commit changed from 3dcfbe90735023422e4f6a79594b0034290adf96 to f8ad239bea1847b5008de00b8ac5ebfbced6f122
Branch pushed to git repo; I updated commit sha1. New commits:
f8ad239  src/setup.py: Do not look for namespace packages in bin, doc

comment:6 Changed 10 months ago by
 Commit changed from f8ad239bea1847b5008de00b8ac5ebfbced6f122 to edbe3d5eb69da6d685365be46c7c3d53d242ae54
comment:7 Changed 10 months ago by
This almost works for me, except for some errors building modules:
[sagelib9.3.beta7] gcc Wnounusedresult Wsigncompare Wunreachablecode fnocommon dynamic DNDEBUG g fwrapv O3 Wall isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk O2 g march=native Isage/libs/ntl I/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/local/lib/python3.9/sitepackages/cysignals Isage/cpython I/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/src I/usr/local/Cellar/python@3.9/3.9.1_8/Frameworks/Python.framework/Versions/3.9/include/python3.9 I/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/local/lib/python3.9/sitepackages/numpy/core/include I/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/local/include I/usr/local/Cellar/python@3.9/3.9.1_8/Frameworks/Python.framework/Versions/3.9/include/python3.9 c sage/libs/ntl/ntl_ZZ.cpp o build/temp.macosx10.15x86_643.9/sage/libs/ntl/ntl_ZZ.o [sagelib9.3.beta7] In file included from sage/libs/ntl/error.cpp:1283: [sagelib9.3.beta7] ../local/include/NTL/tools.h:1064:1: error: unknown type name 'constexpr' [sagelib9.3.beta7] constexpr bool Relocate_aux_has_trivial_copy(T*) [sagelib9.3.beta7] ^
This is probably coming from the fact that the C++ std options that sage_build_cython.create_extension
uses are not used by the new build system.
comment:8 Changed 10 months ago by
 Commit changed from edbe3d5eb69da6d685365be46c7c3d53d242ae54 to 731ff530491af9dfcc46a9d9c226485f2f91caa6
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
804ebd7  sage_setup.dpcbuild.AllBuilder: Restrict workaround to macOS

b4ceee5  sage_setup.docbuild: In the workaround, do not go through build_many to build serially

7369a3e  Merge branch 't/31344/homebrew__docbuild_crashes__libtcl_atforkprepare' into t/31365/add_ntl_to_cython_aliases

ce03e6b  src/sage/schemes/hyperelliptic_curves/hypellfrob.pyx: Merge distutils directives

32576b4  sage.misc.cython: Add NTL aliases, cache result of cython_aliases

6e30e3a  Use variables NTL_INCDIR, NTL_LIBDIR in sage_conf, separate from NTL_PREFIX used in sagebuildenvconfig; set std=c++11 in NTL_CFLAGS

7b1e27b  Merge distutils directives for Cython files using ntl

4d2828d  Switch Cython files that use NTL to language = c++

904a494  Merge branch 't/31365/add_ntl_to_cython_aliases' into t/31377/__configure___enable_editable

731ff53  Switch some Cython files to c++, add some std=c++11

comment:9 Changed 10 months ago by
 Dependencies changed from #30770, #30912, #31357 to #30770, #30912, #31357, #31365
 Status changed from new to needs_review
I have merged #31365, which adds directives to files using NTL; and added a few more distutils directives so that things don't depend so much on the code in sage_build_cython.create_extension
.
comment:10 Changed 10 months ago by
 Description modified (diff)
comment:11 Changed 10 months ago by
 Description modified (diff)
comment:12 Changed 10 months ago by
 Description modified (diff)
comment:13 followup: ↓ 23 Changed 10 months ago by
This seems to reasonably work well here on macOS. There are a small number of doctest failures.
sage t randomseed=0 src/sage/misc/sagedoc.py # 2 doctests failed sage t randomseed=0 src/sage/misc/sageinspect.py # 13 doctests failed sage t randomseed=0 src/sage/repl/interpreter.py # 1 doctest failed sage t randomseed=0 src/sage/categories/primer.py # 1 doctest failed sage t randomseed=0 src/sage/repl/ipython_tests.py # 1 doctest failed sage t randomseed=0 src/sage/sets/set_from_iterator.py # 2 doctests failed sage t randomseed=0 src/sage/interacts/debugger.py # 2 doctests failed sage t randomseed=0 src/sage/misc/lazy_attribute.pyx # 1 doctest failed sage t randomseed=0 src/sage/misc/nested_class.pyx # 1 doctest failed sage t randomseed=0 src/sage/structure/dynamic_class.py # 1 doctest failed sage t randomseed=0 src/sage/all.py # 1 doctest failed sage t randomseed=0 src/sage/misc/edit_module.py # 1 doctest failed sage t randomseed=0 src/sage/cpython/dict_del_by_value.pyx # 2 doctests failed sage t randomseed=0 src/sage/graphs/graph_decompositions/tree_decomposition.pyx # 35 doctests failed
The last one is due to the fact that in the editable install all .pyx
files, even when the modules are not compiled, are available and used for doctesting.
This will need a new solution to suppress the doctests of such modules  see #30778 (sage.doctest.control
: Exclude doctests in files from noninstalled distributions).
comment:14 Changed 10 months ago by
 Cc ghtobiasdiez jhpalmieri chapoton added
comment:15 Changed 10 months ago by
 Commit changed from 731ff530491af9dfcc46a9d9c226485f2f91caa6 to 18df2dc7c27d8e583d5ccdc6acfa39c955b8fa51
Branch pushed to git repo; I updated commit sha1. New commits:
1cb260a  build/pkgs/sagelib/spkginstall: Remove ignoreinstalled

36bbd6c  build/make/Makefile.in (sagelibclean): Remove .so files in editable installs

993c35c  build/pkgs/ntl/spkgconfigure.m4: Fix up

18df2dc  Merge branch 't/31365/add_ntl_to_cython_aliases' into t/31377/__configure___enable_editable

comment:16 Changed 10 months ago by
please stop adding me in cc
comment:17 Changed 10 months ago by
So how does one test this?
comment:18 Changed 10 months ago by
 Description modified (diff)
comment:19 Changed 10 months ago by
I've added quick instructions to the ticket description
comment:20 Changed 10 months ago by
 Reviewers set to Matthias Koeppe, ...
comment:21 Changed 10 months ago by
 Priority changed from major to critical
comment:22 Changed 10 months ago by
The installation of the Jupyter kernel is missing  done by sage_setup.command.sage_install
comment:23 in reply to: ↑ 13 Changed 10 months ago by
Replying to mkoeppe:
This seems to reasonably work well here on macOS. There are a small number of doctest failures.
... sage t randomseed=0 src/sage/graphs/graph_decompositions/tree_decomposition.pyx # 35 doctests failedThe last one is due to the fact that in the editable install all
.pyx
files, even when the modules are not compiled, are available and used for doctesting.
Wrong guess, this is actually #31389.
comment:24 Changed 10 months ago by
 Dependencies changed from #30770, #30912, #31357, #31365 to #30770, #30912, #31357, #31365, #31389
comment:25 Changed 10 months ago by
 Commit changed from 18df2dc7c27d8e583d5ccdc6acfa39c955b8fa51 to f02963b28d939a7f409914a246d74dce7bfc41d4
Branch pushed to git repo; I updated commit sha1. New commits:
19ca17a  src/sage/graphs/graph_decompositions/tdlib/sage_tdlib.cpp: Move one directory level higher

f02963b  Merge branch 't/31389/remove_clash_of_tdlib__with_tdlib_pyx_in__src_sage_graphs_graph_decompositions' into t/31377/__configure___enable_editable

comment:26 Changed 10 months ago by
 Commit changed from f02963b28d939a7f409914a246d74dce7bfc41d4 to 2b293f889f827ec5769ad6ca601294284d9a1f24
Branch pushed to git repo; I updated commit sha1. New commits:
2b293f8  src/setup.py: Install the jupyter kernel using sage_install

comment:27 Changed 10 months ago by
 Status changed from needs_review to needs_work
sage.misc.sageinspect
is broken in an editable build, which causes the docbuild to fail.
comment:28 Changed 10 months ago by
 Commit changed from 2b293f889f827ec5769ad6ca601294284d9a1f24 to 271d42733078659c963550e07970440101a73ab7
comment:29 Changed 10 months ago by
 Commit changed from 271d42733078659c963550e07970440101a73ab7 to f6b2fc4f0f461b2f18aedf49ba238db7a7fdc8b3
Branch pushed to git repo; I updated commit sha1. New commits:
f6b2fc4  src/sage/repl/interpreter.py: Fix a doctest for editable installs of sagelib

comment:30 Changed 10 months ago by
 Status changed from needs_work to needs_review
comment:31 Changed 10 months ago by
 Dependencies changed from #30770, #30912, #31357, #31365, #31389 to #30770, #30912, #31357, #31365, #31389, #31390
comment:32 Changed 10 months ago by
 Commit changed from f6b2fc4f0f461b2f18aedf49ba238db7a7fdc8b3 to e5d47ce731d44f099d635172b34654481eb7e13b
comment:33 Changed 10 months ago by
Now the documentation builds OK and also make ptest
is fine.
comment:34 Changed 10 months ago by
 Status changed from needs_review to needs_work
Can you please include the most recent version of setup.py from #30371. Thanks!
comment:35 followup: ↓ 38 Changed 10 months ago by
 Status changed from needs_work to needs_review
The changes there conflict with the changes done and tested here already.
comment:36 Changed 10 months ago by
(Let's please get this in  unless there are problems on this ticket that need solving here. Otherwise please merge this branch into #30371, resolving the conflicts, and let me know)
comment:37 Changed 10 months ago by
 Cc vdelecroix added
comment:38 in reply to: ↑ 35 ; followup: ↓ 42 Changed 10 months ago by
Replying to mkoeppe:
The changes there conflict with the changes done and tested here already.
What conflicts do you mean? I was referring to the improvements to this code:
+# Exclude a few files if the corresponding distribution is not loaded +optional_packages = ['mcqd', 'bliss', 'tdlib', 'primecount', + 'coxeter3', 'fes', 'sirocco', 'meataxe'] +not_installed_packages = [package for package in optional_packages + if not is_package_installed_and_updated(package)] + +distributions_to_exclude = [f"sage{pkg}" + for pkg in not_installed_packages] +files_to_exclude = filter_cython_sources(SAGE_SRC, distributions_to_exclude) + +log.debug(f"files_to_exclude = {files_to_exclude}") + +python_packages = find_namespace_packages(where=SAGE_SRC, exclude=['bin', 'doc']) +log.debug(f"python_packages = {python_packages}") + +log.info(f"Discovered Python/Cython sources, time: {(time.time()  t):.2f} seconds.") + +# from sage_build_cython: +import Cython.Compiler.Options +Cython.Compiler.Options.embed_pos_in_docstring = True + +try: + log.info("Generating autogenerated sources") + from sage_setup.autogen import autogen_all + autogen_all() + + from Cython.Build import cythonize + from sage.env import cython_aliases, sage_include_directories + extensions = cythonize( + ["**/*.pyx"], + exclude=files_to_exclude, + include_path=sage_include_directories(use_sources=True) + ['.'], + compile_time_env=compile_time_env_variables(), + compiler_directives=compiler_directives(False), + aliases=cython_aliases(), + create_extension=create_extension, + nthreads=4) +except Exception as exception: + log.warn(f"Exception while generating and cythonizing source files: {exception}") + extensions = None
This has a few problems/issues:
 The first few lines still raise an exception if Cython (and maybe other packages) are not installed.
 You want to autogenerate code before filtering it / excluding parts of it.
 The exception catch is to general and was meant to only capture module import errors.
All these things are fixed in #30371.
comment:39 Changed 10 months ago by
OK, fine, I'll do the merging.
comment:40 followup: ↓ 41 Changed 10 months ago by
Concerning this change:
# distutils: language = c++ +# distutils: extra_compile_args = std=c++11
Woudn't it make sense to specify std=c++11
automatically once language = c++
is set? In fact there is code that looks like it is doing this job in sage_build_cython.
I think also the cython files in libs/singular need similar changes (at least in #30371 errors are thrown otherwise).
comment:41 in reply to: ↑ 40 ; followup: ↓ 43 Changed 10 months ago by
Replying to ghtobiasdiez:
Concerning this change:
# distutils: language = c++ +# distutils: extra_compile_args = std=c++11Woudn't it make sense to specify
std=c++11
automatically oncelanguage = c++
is set? In fact there is code that looks like it is doing this job in sage_build_cython.
Of course, but you decided not to use the existing code. So I have made the changes that make it work.
comment:42 in reply to: ↑ 38 Changed 10 months ago by
Important to keep in mind that I have created this ticket so that we can ship something in Sage 9.3 that works. So let's please keep focused on what is described here on the ticket description.
Replying to ghtobiasdiez:
 The first few lines still raise an exception if Cython (and maybe other packages) are not installed.
Not important for this ticket; we do have Cython.
 You want to autogenerate code before filtering it / excluding parts of it.
Why? Even after modularization, there will be exactly 1 module that will contain the autogenerated files.
comment:43 in reply to: ↑ 41 Changed 10 months ago by
Replying to mkoeppe:
Replying to ghtobiasdiez:
Concerning this change:
# distutils: language = c++ +# distutils: extra_compile_args = std=c++11Woudn't it make sense to specify
std=c++11
automatically oncelanguage = c++
is set? In fact there is code that looks like it is doing this job in sage_build_cython.Of course, but you decided not to use the existing code. So I have made the changes that make it work.
I couldn't make it work with the sage_build_cython command, yes. So changing create_extension
in sage_setup/extensions.py
would be a better solution then?
comment:44 followup: ↓ 62 Changed 10 months ago by
Your approach of starting the build system from scratch has certainly been valuable, so that we can see what parts of the code are needed.
In terms of complexity of code, I think adding the small of number of explicit directives to the files may actually be preferable to the more complex code regarding the std
flags in sage_build_cython
.
Let's try it out as is.
In some followup ticket, a more general solution may be needed at some point. Especially when use of c++14/17 becomes more widespread, then fixing things to std=c++11
may no longer be the right thing.
Let's do this when it becomes an issue; not now.
comment:45 Changed 10 months ago by
 Cc isuruf added
comment:46 Changed 10 months ago by
 Cc ghkliem added
comment:47 Changed 10 months ago by
Does this now instead implement ./configure enablebuildasroot
? It sounds like that's related to sudo make
; is that what's intended?
comment:48 Changed 10 months ago by
Thanks for catching the typo in the help string.
comment:49 Changed 10 months ago by
 Commit changed from e5d47ce731d44f099d635172b34654481eb7e13b to 50f5010883bee3f2fc459848f2dac68d699bce87
Branch pushed to git repo; I updated commit sha1. New commits:
50f5010  build/pkgs/sagelib/spkgconfigure.m4: Fix help string of enableeditable

comment:50 followups: ↓ 51 ↓ 60 Changed 10 months ago by
I like it. Sometimes, I wanted to look into a C/C++ file that cython generated and I just couldn't locate it recently. No it's sitting all there for you.
What vanished though is the progress report, how many files yet need to be compiled. I like this feature, because it tells you whether you should wait on it or whether you should carry on and do something else.
comment:51 in reply to: ↑ 50 Changed 10 months ago by
Replying to ghkliem:
What vanished though is the progress report, how many files yet need to be compiled. I like this feature, because it tells you whether you should wait on it or whether you should carry on and do something else.
Tobias wrote this as a minimal version using standard setuptools + cython. Let's keep adding additional features of this kind in mind for possible followup tickets.
comment:52 Changed 10 months ago by
Ok. Just mentioned that this is missing and I like it back, if possible. I think I would even switch to this, without the counter. Just because it is nice to have the files where you think the should be.
You introduced a trailing whitespace in src/sage_setup/find.py
.
comment:53 followup: ↓ 56 Changed 10 months ago by
Why is it python3
and not $PYTHON
?
+if [ "$SAGE_EDITABLE" = yes ]; then
+ time python3 m pip install verbose nodeps noindex nobuildisolation isolated editable .  exit 1
+else
+ time "$PYTHON" u setup.py nousercfg build install  exit 1
+fi
+
comment:54 Changed 10 months ago by
You freshly introduced from distutils.command.build_ext import build_ext
in /src/sage_setup/command/sage_build_ext_minimal.py
.
I think this should be replaced by setuptools somehow or at least import setuptools before as to get rid of setuptools.
comment:55 Changed 10 months ago by
We should probably merge #30984 too.
comment:56 in reply to: ↑ 53 Changed 10 months ago by
Replying to ghkliem:
Why is it
python3
and not$PYTHON
?+if [ "$SAGE_EDITABLE" = yes ]; then + time python3 m pip install verbose nodeps noindex nobuildisolation isolated editable .  exit 1 +else + time "$PYTHON" u setup.py nousercfg build install  exit 1 +fi +
Thanks for spotting this inconsistency. Actually, both should be python3
(since #30731, which replaced sagepython23
).
comment:57 Changed 10 months ago by
 Commit changed from 50f5010883bee3f2fc459848f2dac68d699bce87 to 8c5e9c3b56070a9a1ef1040555da83365908f286
Branch pushed to git repo; I updated commit sha1. New commits:
8c5e9c3  build/pkgs/sagelib/spkginstall: python3, not $PYTHON

comment:58 Changed 10 months ago by
 Commit changed from 8c5e9c3b56070a9a1ef1040555da83365908f286 to 0df54f76dee2089b5c14e3db8f5ef8c5a2d04796
Branch pushed to git repo; I updated commit sha1. New commits:
0df54f7  src/sage_setup/find.py: Fix whitespace

comment:59 Changed 10 months ago by
 Commit changed from 0df54f76dee2089b5c14e3db8f5ef8c5a2d04796 to a63e6157874d8be7e29b078d646936142f4bdcea
comment:60 in reply to: ↑ 50 Changed 10 months ago by
Replying to ghkliem:
What vanished though is the progress report, how many files yet need to be compiled. I like this feature, because it tells you whether you should wait on it or whether you should carry on and do something else.
I think a progress bar will not be easy. The new version directly uses the cythonize method, which doesn't seem to have any configuration in this direction, see https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html?highlight=cythonize#cythonizearguments.
comment:61 Changed 10 months ago by
I have created #31406 as a place for followup items.
comment:62 in reply to: ↑ 44 Changed 10 months ago by
Replying to mkoeppe:
In terms of complexity of code, I think adding the small of number of explicit directives to the files may actually be preferable to the more complex code regarding the
std
flags insage_build_cython
. Let's try it out as is.
Ok, sounds good to me. But notice that explicitly specifying the std
changes the behavior on cygwin (for a "normal" build) since the following code in sage_build_cython is no longer invoked:
if sys.platform == 'cygwin': # Cygwin (particularly newlib, Cygwin's libc) has some bugs # with strict ANSI C/C++ in some headers; using the GNU # extensions typically fares better: # https://trac.sagemath.org/ticket/24192 if cplusplus: cflags.append("std=gnu++11") else: cflags.append("std=gnu99")
I don't know if this may have negative side effects.
comment:63 followup: ↓ 65 Changed 10 months ago by
Thanks for pointing this out; we'll see when we add automated tests for enableeditable
to the Cygwin GH Actions. (in a ticket after #31064 is merged)
comment:64 followups: ↓ 66 ↓ 67 ↓ 69 ↓ 70 Changed 10 months ago by
A few more observations:
 There is currently no documentation for how to use the inplace install (and why).
 Running the configure command yields the following warning at the end:
.... config.status: creating directory local/var/lib/sage/installed configure: WARNING: unrecognized options: enableeditable configure: notice: the following SPKGs did not find equivalent system packages ...
 My only means to test this on mac is with the github action added in #30371. Admittedly, this is using a lightly different setup, but since the sage inplace install fails, I wanted to point this out. First of all, the files in
sage/libs/singular
don't specify that they should be compiled withlanguage=c++
andstd=c++11
. Secondly, there are many errors of the form/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/nl_types.h:97:1: error: unknown type name 'nl_catd' nl_catd catopen(const char *, int); ^
See e.g. https://github.com/tobiasdiez/sage/runs/1893534204?check_suite_focus=true
 On Linux (with wsl), the make build fails with
[pplpy0.8.6] Copying package files from temporary location /mnt/d/Programming/sage/local/var/tmp/sage/build/pplpy0.8.6/inst to /mnt/d/Programming/sage/local [pplpy0.8.6] Running postinstall script for pplpy0.8.6. [pplpy0.8.6] Traceback (most recent call last): [pplpy0.8.6] File "/mnt/d/Programming/sage/local/bin/sphinxbuild", line 5, in <module> [pplpy0.8.6] from sphinx.cmd.build import main [pplpy0.8.6] File "/mnt/d/Programming/sage/local/lib/python3.8/sitepackages/sphinx/cmd/build.py", line 25, in <module> [pplpy0.8.6] from sphinx.application import Sphinx [pplpy0.8.6] File "/mnt/d/Programming/sage/local/lib/python3.8/sitepackages/sphinx/application.py", line 31, in <module> [pplpy0.8.6] from sphinx.config import Config [pplpy0.8.6] File "/mnt/d/Programming/sage/local/lib/python3.8/sitepackages/sphinx/config.py", line 28, in <module> [pplpy0.8.6] from sphinx.util.tags import Tags [pplpy0.8.6] File "/mnt/d/Programming/sage/local/lib/python3.8/sitepackages/sphinx/util/tags.py", line 11, in <module> [pplpy0.8.6] from jinja2 import nodes [pplpy0.8.6] File "/mnt/d/Programming/sage/local/lib/python3.8/sitepackages/jinja2/__init__.py", line 6, in <module> [pplpy0.8.6] from markupsafe import escape [pplpy0.8.6] ValueError: source code string cannot contain null bytes [pplpy0.8.6] make[5]: *** [Makefile:21: html] Error 1 [pplpy0.8.6] cp: cannot stat 'build/html/*': No such file or directory
That's probably not related to this ticket, but prevents me from testing it.
comment:65 in reply to: ↑ 63 ; followup: ↓ 68 Changed 10 months ago by
Replying to mkoeppe:
Thanks for pointing this out; we'll see when we add automated tests for
enableeditable
to the Cygwin GH Actions. (in a ticket after #31064 is merged)
The point is that explicitly specifying std
in the cython file also overrides it for the sagedistribution compilation, i.e. the one using the setup.py in build.
comment:66 in reply to: ↑ 64 ; followup: ↓ 71 Changed 10 months ago by
Replying to ghtobiasdiez:
 Running the configure command yields the following warning at the end:
.... config.status: creating directory local/var/lib/sage/installed configure: WARNING: unrecognized options: enableeditable
Forgot to run ./bootstrap
?
comment:67 in reply to: ↑ 64 Changed 10 months ago by
Replying to ghtobiasdiez:
 My only means to test this on mac is with the github action added in #30371.
Let's please not pull a discussion of these experimental GH actions from #30371 into this ticket.
If we add automatic testing, we will do it on the basis of the existing GH actions.
comment:68 in reply to: ↑ 65 Changed 10 months ago by
Replying to ghtobiasdiez:
Replying to mkoeppe:
Thanks for pointing this out; we'll see when we add automated tests for
enableeditable
to the Cygwin GH Actions. (in a ticket after #31064 is merged)The point is that explicitly specifying
std
in the cython file also overrides it for the sagedistribution compilation, i.e. the one using the setup.py in build.
Yes, it is a change and we are using portability testing to make sure that it does not break anything on the supported platforms.
comment:69 in reply to: ↑ 64 Changed 10 months ago by
Replying to ghtobiasdiez:
A few more observations:
 There is currently no documentation for how to use the inplace install (and why).
Well, I hope we can merge this ticket into the next beta and then ask developers to try it. We can document it in a followup ticket.
comment:70 in reply to: ↑ 64 Changed 10 months ago by
Replying to ghtobiasdiez:
 On Linux (with wsl), the make build fails with
[pplpy0.8.6] Copying package files from temporary location /mnt/d/Programming/sage/local/var/tmp/sage/build/pplpy0.8.6/inst to /mnt/d/Programming/sage/local [pplpy0.8.6] Running postinstall script for pplpy0.8.6. [pplpy0.8.6] from markupsafe import escape [pplpy0.8.6] ValueError: source code string cannot contain null bytes [pplpy0.8.6] make[5]: *** [Makefile:21: html] Error 1 [pplpy0.8.6] cp: cannot stat 'build/html/*': No such file or directoryThat's probably not related to this ticket, but prevents me from testing it.
I have not seen this before and I don't think it has anything to with the present ticket. Please check if this also occurs with 9.3.beta7 and open a separate ticket if this problem is reproducible.
comment:71 in reply to: ↑ 66 Changed 10 months ago by
comment:72 Changed 10 months ago by
I'm getting a few doctest failures:
sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py ********************************************************************** File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter Failed example: print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test Expected: dummy line ... ZeroDivisionError...Traceback (most recent call last) <ipythoninput...> in <module>... > 1 Integer(1)/Integer(0) .../sage/rings/integer.pyx in sage.rings.integer.Integer...div... (.../sage/rings/integer.c:...)() ... > ... raise ZeroDivisionError("rational division by zero") ... x = <Rational> Rational.__new__(Rational) ... mpq_div_zz(x.value, ....value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero Got: dummy line  ZeroDivisionError Traceback (most recent call last) <ipythoninput172ac74c5f414> in <module> > 1 Integer(1)/Integer(0) <BLANKLINE> /srv/public/kliem/sage/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__truediv__() 2038 if type(left) is type(right): 2039 if mpz_sgn((<Integer>right).value) == 0: > 2040 raise ZeroDivisionError("rational division by zero") 2041 x = <Rational> Rational.__new__(Rational) 2042 mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero ********************************************************************** 1 item had failures: 1 of 20 in sage.repl.interpreter [141 tests, 1 failure, 1.97 s]  sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py # 1 doctest failed  Total time for all tests: 7.4 seconds cpu time: 0.6 seconds cumulative wall time: 2.0 seconds kliem@lech:~/localhome/sage$ sage t long randomseed=0 src/sage/stats/hmm/hmm.pyx # 1 doctest failed Running doctests with ID 2021021708074053dddc3c. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage/stats/hmm/hmm.pyx ********************************************************************** File "src/sage/stats/hmm/hmm.pyx", line 1234, in sage.stats.hmm.hmm.DiscreteHiddenMarkovModel.baum_welch Failed example: m.baum_welch(v,fix_emissions=True) Expected: (66.98630856918774, 100) Got: (66.98630856918776, 100) ********************************************************************** 1 item had failures: 1 of 14 in sage.stats.hmm.hmm.DiscreteHiddenMarkovModel.baum_welch [121 tests, 1 failure, 0.32 s]  sage t long warnlong 62.2 randomseed=0 src/sage/stats/hmm/hmm.pyx # 1 doctest failed  Total time for all tests: 0.4 seconds cpu time: 0.3 seconds cumulative wall time: 0.3 seconds kliem@lech:~/localhome/sage$ sage t long randomseed=0 src/sage_setup/find.py # 6 doctests failed Running doctests with ID 20210217080742447cd574. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage_setup/find.py ********************************************************************** File "src/sage_setup/find.py", line 199, in sage_setup.find.filter_cython_sources Failed example: 'sage/graphs/graph_decompositions/tdlib.pyx' in cython_modules Expected: True Got: False ********************************************************************** File "src/sage_setup/find.py", line 264, in sage_setup.find.find_extra_files Failed example: extras["sage/ext/interpreters"] Expected: ['.../src/sage/ext/interpreters/wrapper_cdf.pxd', ...wrapper_cdf.h...] Got: ['/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_py.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cdf.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_el.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rdf.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cc.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rr.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_py.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cdf.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rr.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rdf.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cc.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_el.pxd'] ********************************************************************** File "src/sage_setup/find.py", line 324, in sage_setup.find.installed_files_by_module Failed example: (f,) = files_by_module['sage.structure.sage_object']; f Exception raised: Traceback (most recent call last): File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 714, in _run self.compile_and_execute(example, compiler, test.globs) File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 1133, in compile_and_execute exec(compiled, globs) File "<doctest sage_setup.find.installed_files_by_module[5]>", line 1, in <module> (f,) = files_by_module['sage.structure.sage_object']; f ValueError: not enough values to unpack (expected 1, got 0) ********************************************************************** File "src/sage_setup/find.py", line 326, in sage_setup.find.installed_files_by_module Failed example: (f1, f2) = sorted(files_by_module['sage.structure']) Exception raised: Traceback (most recent call last): File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 714, in _run self.compile_and_execute(example, compiler, test.globs) File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 1133, in compile_and_execute exec(compiled, globs) File "<doctest sage_setup.find.installed_files_by_module[6]>", line 1, in <module> (f1, f2) = sorted(files_by_module['sage.structure']) ValueError: not enough values to unpack (expected 2, got 0) ********************************************************************** File "src/sage_setup/find.py", line 327, in sage_setup.find.installed_files_by_module Failed example: f1 Exception raised: Traceback (most recent call last): File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 714, in _run self.compile_and_execute(example, compiler, test.globs) File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 1133, in compile_and_execute exec(compiled, globs) File "<doctest sage_setup.find.installed_files_by_module[7]>", line 1, in <module> f1 NameError: name 'f1' is not defined ********************************************************************** File "src/sage_setup/find.py", line 329, in sage_setup.find.installed_files_by_module Failed example: f2 Exception raised: Traceback (most recent call last): File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 714, in _run self.compile_and_execute(example, compiler, test.globs) File "/srv/public/kliem/sage/src/sage/doctest/forker.py", line 1133, in compile_and_execute exec(compiled, globs) File "<doctest sage_setup.find.installed_files_by_module[8]>", line 1, in <module> f2 NameError: name 'f2' is not defined ********************************************************************** 3 items had failures: 1 of 7 in sage_setup.find.filter_cython_sources 1 of 7 in sage_setup.find.find_extra_files 4 of 11 in sage_setup.find.installed_files_by_module [42 tests, 6 failures, 0.40 s]  sage t long warnlong 62.2 randomseed=0 src/sage_setup/find.py # 6 doctests failed  Total time for all tests: 0.4 seconds cpu time: 0.4 seconds cumulative wall time: 0.4 seconds
kliem@lech:~/localhome/sage$ sage t long randomseed=0 src/sage/tests/cmdline.py # 3 doctests failed Running doctests with ID 202102170807447353eba5. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage/tests/cmdline.py ********************************************************************** File "src/sage/tests/cmdline.py", line 586, in sage.tests.cmdline.test_executable Failed example: out.startswith("3.") Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 588, in sage.tests.cmdline.test_executable Failed example: err Expected: '' Got: '/srv/public/kliem/sage/src/bin/sage: line 588: exec: sqlite3: not found\n' ********************************************************************** File "src/sage/tests/cmdline.py", line 590, in sage.tests.cmdline.test_executable Failed example: ret Expected: 0 Got: 127 ********************************************************************** 1 item had failures: 3 of 231 in sage.tests.cmdline.test_executable [230 tests, 3 failures, 21.87 s]  sage t long warnlong 62.2 randomseed=0 src/sage/tests/cmdline.py # 3 doctests failed 
The test in src/sage/tests/cmdline.py
is just there for completeness and can be ignored.
comment:73 Changed 10 months ago by
I got failures in repl/interpreter.py
and sage_setup/find.py
.
comment:74 Changed 10 months ago by
 Commit changed from a63e6157874d8be7e29b078d646936142f4bdcea to 78a1587c275e5528a9e020f926302851647da6d4
Branch pushed to git repo; I updated commit sha1. New commits:
78a1587  src/setup.py: Add . to sys.path so that pyproject.toml from #30913 does not break the build

comment:75 Changed 10 months ago by
 Commit changed from 78a1587c275e5528a9e020f926302851647da6d4 to ccce09e2c8cc7fa802ed18450778096f76364da7
Branch pushed to git repo; I updated commit sha1. New commits:
ccce09e  build/pkgs/sagelib/spkginstall [SAGE_EDITABLE=yes]: Uninstall distutilsinstalled sagelib

comment:76 Changed 10 months ago by
 Commit changed from ccce09e2c8cc7fa802ed18450778096f76364da7 to 5a918778123e677c2209425bdc9905b45699ab6e
Branch pushed to git repo; I updated commit sha1. New commits:
5a91877  sage_setup.find: Fix filter_cython_sources doctests

comment:77 Changed 10 months ago by
The remaining failures in sage_setup/find.py
are now all about installed_files_by_module
; I'll have to see what to do about them (this function is only needed for the old build system).
The failure in sage/repl/interpreter.py
seems to indicate that the C source information (sage/rings/integer.c
) is missing.
comment:78 Changed 10 months ago by
 Commit changed from 5a918778123e677c2209425bdc9905b45699ab6e to e272614fd26ceb3992754a014099fc644d470f06
comment:79 Changed 10 months ago by
 Commit changed from e272614fd26ceb3992754a014099fc644d470f06 to 67072f34146e7ed547f241854755a462660b53cf
Branch pushed to git repo; I updated commit sha1. New commits:
67072f3  src/setup.py: Use include, not exclude in find_namespace_packages to get top_level info right

comment:80 Changed 10 months ago by
 Commit changed from 67072f34146e7ed547f241854755a462660b53cf to 0bcf8a40008300ad2fca24310a052b0b2fc6b510
Branch pushed to git repo; I updated commit sha1. New commits:
0bcf8a4  build/pkgs/sagelib/spkginstall: When switching from editable install to traditional install, remove the egglink

comment:81 Changed 10 months ago by
 Commit changed from 0bcf8a40008300ad2fca24310a052b0b2fc6b510 to 981124cd020eaa4127a9ae0e6c9009269e45a2ff
Branch pushed to git repo; I updated commit sha1. New commits:
981124c  sage_setup.find.installed_files_by_module: Make doctest work for editable installs too

comment:82 Changed 10 months ago by
This fixes the remaining doctests for me.
comment:83 Changed 10 months ago by
Not for me:
I still get src/sage/tests/cmdline.py
, which can be ignored.
On top:
kliem@lech:~/localhome/sage$ sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py Running doctests with ID 202102180750515ae2b230. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py ********************************************************************** File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter Failed example: print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test Expected: dummy line ... ZeroDivisionError...Traceback (most recent call last) <ipythoninput...> in <module>... > 1 Integer(1)/Integer(0) .../sage/rings/integer.pyx in sage.rings.integer.Integer...div... (.../sage/rings/integer.c:...)() ... > ... raise ZeroDivisionError("rational division by zero") ... x = <Rational> Rational.__new__(Rational) ... mpq_div_zz(x.value, ....value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero Got: dummy line  ZeroDivisionError Traceback (most recent call last) <ipythoninput172ac74c5f414> in <module> > 1 Integer(1)/Integer(0) <BLANKLINE> /srv/public/kliem/sage/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__truediv__() 2038 if type(left) is type(right): 2039 if mpz_sgn((<Integer>right).value) == 0: > 2040 raise ZeroDivisionError("rational division by zero") 2041 x = <Rational> Rational.__new__(Rational) 2042 mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero ********************************************************************** 1 item had failures: 1 of 20 in sage.repl.interpreter [141 tests, 1 failure, 1.94 s]  sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py # 1 doctest failed  Total time for all tests: 7.3 seconds cpu time: 0.6 seconds cumulative wall time: 1.9 seconds kliem@lech:~/localhome/sage$ sage t long warnlong 62.2 randomseed=0 src/sage/stats/hmm/hmm.pyx Running doctests with ID 2021021807510857678760. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage/stats/hmm/hmm.pyx ********************************************************************** File "src/sage/stats/hmm/hmm.pyx", line 1234, in sage.stats.hmm.hmm.DiscreteHiddenMarkovModel.baum_welch Failed example: m.baum_welch(v,fix_emissions=True) Expected: (66.98630856918774, 100) Got: (66.98630856918776, 100) ********************************************************************** 1 item had failures: 1 of 14 in sage.stats.hmm.hmm.DiscreteHiddenMarkovModel.baum_welch [121 tests, 1 failure, 0.33 s]  sage t long warnlong 62.2 randomseed=0 src/sage/stats/hmm/hmm.pyx # 1 doctest failed  Total time for all tests: 0.4 seconds cpu time: 0.3 seconds cumulative wall time: 0.3 seconds kliem@lech:~/localhome/sage$ sage t long warnlong 62.2 randomseed=0 src/sage_setup/find.py Running doctests with ID 20210218075121d3281c6c. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage_setup/find.py ********************************************************************** File "src/sage_setup/find.py", line 264, in sage_setup.find.find_extra_files Failed example: extras["sage/ext/interpreters"] Expected: ['.../src/sage/ext/interpreters/wrapper_cdf.pxd', ...wrapper_cdf.h...] Got: ['/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_py.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cdf.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_el.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rdf.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cc.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rr.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_py.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cdf.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rr.pyx', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_rdf.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_cc.pxd', '/srv/public/kliem/sage/src/sage/ext/interpreters/wrapper_el.pxd'] ********************************************************************** 1 item had failures: 1 of 7 in sage_setup.find.find_extra_files [41 tests, 1 failure, 0.47 s]  sage t long warnlong 62.2 randomseed=0 src/sage_setup/find.py # 1 doctest failed  Total time for all tests: 0.5 seconds cpu time: 0.5 seconds cumulative wall time: 0.5 seconds
comment:84 Changed 10 months ago by
find_extra_files
does exactly what the documentation claims it does, it lists all pyx, pxd, pxi
files.
comment:85 Changed 10 months ago by
After make sagelibclean sagelib
I can reproduce this error too. the .h
files are missing in the list.
comment:86 followup: ↓ 90 Changed 10 months ago by
My guess is that the doctest failure in src/sage/repl/interpreter.py
will go away if you do make sagelibclean
comment:87 Changed 10 months ago by
The missing .h
files are by design. This function gives only .h.
files in cythonized
, which is pointless now. I don't know what would be the best way to deal with this.
Can you fix the precision error in src/sage/stats/hmm/hmm.pyx
as well (# abs tol 1e13
). It seems pointless to have an extra ticket for this.
comment:88 followup: ↓ 92 Changed 10 months ago by
Ran make sagelibclean sagelib
and make build
and it still persists:
kliem@lech:~/localhome/sage$ sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py Running doctests with ID 202102180859139182572c. Git branch: test_31377 Using optional=build,debian,dochtml,memlimit,pip,sage,sage_spkg Doctesting 1 file. sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py ********************************************************************** File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter Failed example: print("dummy line"); shell.run_cell('1/0') # see #25320 for the reason of the `...` and the dummy line in this test Expected: dummy line ... ZeroDivisionError...Traceback (most recent call last) <ipythoninput...> in <module>... > 1 Integer(1)/Integer(0) .../sage/rings/integer.pyx in sage.rings.integer.Integer...div... (.../sage/rings/integer.c:...)() ... > ... raise ZeroDivisionError("rational division by zero") ... x = <Rational> Rational.__new__(Rational) ... mpq_div_zz(x.value, ....value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero Got: dummy line  ZeroDivisionError Traceback (most recent call last) <ipythoninput172ac74c5f414> in <module> > 1 Integer(1)/Integer(0) <BLANKLINE> /srv/public/kliem/sage/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__truediv__() 2038 if type(left) is type(right): 2039 if mpz_sgn((<Integer>right).value) == 0: > 2040 raise ZeroDivisionError("rational division by zero") 2041 x = <Rational> Rational.__new__(Rational) 2042 mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value) <BLANKLINE> ZeroDivisionError: rational division by zero ********************************************************************** 1 item had failures: 1 of 20 in sage.repl.interpreter [141 tests, 1 failure, 2.90 s]  sage t long warnlong 62.2 randomseed=0 src/sage/repl/interpreter.py # 1 doctest failed  Total time for all tests: 8.4 seconds cpu time: 0.6 seconds cumulative wall time: 2.9 seconds
comment:89 Changed 10 months ago by
I mean it makes completely sense that the cythonized file is no longer written in parenthesis, because it is in the obvious location. So no need to specify this anymore.
comment:90 in reply to: ↑ 86 ; followup: ↓ 93 Changed 10 months ago by
Replying to mkoeppe:
My guess is that the doctest failure in
src/sage/repl/interpreter.py
will go away if you domake sagelibclean
I think this problem comes from the fact that currently find_packages is called before the autogeneration. Thus, the autogenerated files are not included in the cythonization step. This is fixed in the setup.py in #30371.
comment:91 Changed 10 months ago by
I think we can leave those failures for later fix, as long as we take care of them at some point.
None of the doctest failures is serious.
I guess this isn't ready yet for being used for patchbots.
comment:92 in reply to: ↑ 88 Changed 10 months ago by
Replying to ghkliem:
Ran
make sagelibclean sagelib
andmake build
and it still persists:
I'm pretty sure I fixed this with e272614; perhaps make sagelibclean
still does not clean enough?
comment:93 in reply to: ↑ 90 Changed 10 months ago by
Replying to ghtobiasdiez:
Replying to mkoeppe:
My guess is that the doctest failure in
src/sage/repl/interpreter.py
will go away if you domake sagelibclean
I think this problem comes from the fact that currently find_packages is called before the autogeneration. Thus, the autogenerated files are not included in the cythonization step. This is fixed in the setup.py in #30371.
No, calling autogen earlier does not help.
comment:94 Changed 10 months ago by
 Description modified (diff)
comment:95 followup: ↓ 97 Changed 10 months ago by
For changes in Python source, one might even use importlib
and not restart sage at all!
comment:96 Changed 10 months ago by
 Commit changed from 981124cd020eaa4127a9ae0e6c9009269e45a2ff to 8b5d51bd0ff4823972f95f0728eba485d3443afe
Branch pushed to git repo; I updated commit sha1. New commits:
8b5d51b  src/sage_setup: Generalize doctests to editable install using new function _cythonized_dir

comment:97 in reply to: ↑ 95 Changed 10 months ago by
Replying to vdelecroix:
For changes in Python source, one might even use
importlib
and not restart sage at all!
True, but let's not document this  too many possible pitfalls related to class redefinitions
comment:98 Changed 10 months ago by
(Too bad that Python does not have UPDATEINSTANCEFORREDEFINEDCLASS
http://clhs.lisp.se/Body/f_upda_1.htm)
comment:99 Changed 10 months ago by
Ready for another round of testing. (I won't do the unrelated hmm.pyx
doctest fix on this ticket)
comment:100 Changed 10 months ago by
After deleting everything and reinstalling it, I still get the failure in src/sage/repl/interpreter.py
. The find.py
thing is gone now.
comment:101 Changed 10 months ago by
I'm getting the src/sage/repl/interpreter.py
failure now too again... any help with tracking this down is welcome...
comment:102 Changed 10 months ago by
Or we can leave this issue for the followup ticket #31406, that would be fine with me too.
comment:103 followup: ↓ 106 Changed 10 months ago by
If we're to believe the doctest language ("Check that Cython source code appears in tracebacks"), then this is functioning properly, it's just missing "(.../cythonized/sage/rings/integer.c:...)()". Can we just replace that part of the doctest with "...", or is it important?

src/sage/repl/interpreter.py
diff git a/src/sage/repl/interpreter.py b/src/sage/repl/interpreter.py index befa75fc38..3bd894851d 100644
a b Check that Cython source code appears in tracebacks:: 80 80 ZeroDivisionError...Traceback (most recent call last) 81 81 <ipythoninput...> in <module>... 82 82 > 1 Integer(1)/Integer(0) 83 .../sage/rings/integer.pyx in sage.rings.integer.Integer...div... (.../sage/rings/integer.c:...)()83 .../sage/rings/integer.pyx in sage.rings.integer.Integer...div... 84 84 ... 85 85 > ... raise ZeroDivisionError("rational division by zero") 86 86 ....: x = <Rational> Rational.__new__(Rational)
By the way, git status
tells me
Untracked files: (use "git add <file>..." to include in what will be committed) src/cython_debug/
comment:104 Changed 10 months ago by
I had also these untracked files in src/cython_debug.
The tests seem to pass in general for me. However, they need a lot of ram, like > 25GB which sometimes leads to a crash of the system. In particular, the tests around sage/plot seem to very memeroy intensive (but the memory doesn't go down afterwards). Is this normal? I could also reproduce this on the github actions in #30371 which also crash reproducible after about 2 hours of doctesting.
comment:105 Changed 10 months ago by
 Commit changed from 8b5d51bd0ff4823972f95f0728eba485d3443afe to eb631b23bc8f207a57da1550cf799de86983bd94
comment:106 in reply to: ↑ 103 Changed 10 months ago by
 Cc tscrim added
Replying to jhpalmieri:
it's just missing "(.../cythonized/sage/rings/integer.c:...)()". Can we just replace that part of the doctest with "...", or is it important?
I don't know if this information is important, perhaps someone who programs Cython can tell us?
comment:107 Changed 10 months ago by
Using git blame, this doctest was introduced in commit 6e7ad656ba3307ff67949e251550c101758f9963
. In this commit Jeroen Demeyer added a IPython patch. Of course this patch has long merged upstream and has now changed several times and I failed to located the place, where things are happening.
However, things are out of our control.
I think the reason why "(.../cythonized/sage/rings/integer.c:...)()" appears is that the cythonized file is located at a different location then the source file. So this might be important, when you are debugging. However, if the cythonized files are no longer hidden somewhere else, then it appears pointless to print this information and this is probably why IPython doesn't do it. I mean the file is right next to the other file. It is redundant information in this case.
Long story short, I think this can go. It is an IPython feature of which we do not have control anyway. What is important, is that the corresponding pyx
file appears correctly, which is the case.
comment:108 Changed 10 months ago by
 Commit changed from eb631b23bc8f207a57da1550cf799de86983bd94 to daec3e803e8f6a6de8e717459c31b2b7de7a8906
Branch pushed to git repo; I updated commit sha1. New commits:
daec3e8  src/sage/repl/interpreter.py: Do not test that IPython displays the location of the Cythongenerated .c file

comment:109 Changed 10 months ago by
Thanks for investigating. This is sufficiently convincing, so I have removed the test.
comment:110 Changed 10 months ago by
Ok, all tests pass now (except for those two unrelated tests).
comment:111 Changed 10 months ago by
Any further concerns, or can we get this in?
comment:112 Changed 10 months ago by
 Reviewers changed from Matthias Koeppe, ... to Matthias Koeppe, Tobias Diez, ...
From my side this looks good.
comment:113 Changed 10 months ago by
I'm happy with it, but didn't have the time to understand everything yet.
comment:114 Changed 9 months ago by
 Reviewers changed from Matthias Koeppe, Tobias Diez, ... to Matthias Koeppe, Tobias Diez, Jonathan Kliem
 Status changed from needs_review to positive_review
LGTM.
I don't know if setting nthreads=4
for cythonizing is perfect. Maybe this could be picked up somewhere in a follow up ticket. E.g. if the environment variable MAKE
is set to make ... jN ...
we should probably respect this.
comment:115 Changed 9 months ago by
Thanks! I've added your suggestion to #31406.
comment:116 Changed 9 months ago by
Thanks for the review!
comment:117 Changed 9 months ago by
 Commit changed from daec3e803e8f6a6de8e717459c31b2b7de7a8906 to 8b3f3905c884f86293f103fe5bbb7a78c3d7ce02
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
8b3f390  Merge tag '9.3.beta8' into t/31377/__configure___enable_editable

comment:118 Changed 9 months ago by
 Status changed from needs_review to positive_review
comment:119 Changed 9 months ago by
 Branch changed from u/mkoeppe/__configure___enable_editable to 8b3f3905c884f86293f103fe5bbb7a78c3d7ce02
 Resolution set to fixed
 Status changed from positive_review to closed
comment:120 Changed 9 months ago by
 Commit 8b3f3905c884f86293f103fe5bbb7a78c3d7ce02 deleted
I finally tracked this ticket has the breaking point of the way I build the documentation in sageongentoo. This ticket not #30010.
Unlike the current philosophy pursued by vanilla sage in distro we usually build documentation before installing. There are philosophy discussions to be had about the approaches. I am sometimes not even completely sure how things work. But here the reason is simple, before this ticket, all the sources where copied under build/lib/
during the build process. After this ticket only the final compiled cython modules are found under build/lib/
. Historically during the documentation building in sageongentoo the just compiled copy of sagelib under build/lib/
was used. This is not possible anymore.
Is there a switch to pass to setup.py
that would copy the source in build/lib
after this ticket?
comment:121 Changed 9 months ago by
I don't think this ticket changes anything if you don't use configure enableeditable
.
comment:122 Changed 9 months ago by
Just be sure to use build/pkgs/sagelib/src/setup.py
, not src/setup.py
. This builds in build/pkgs/sagelib/src/build/
.
comment:123 followup: ↓ 124 Changed 9 months ago by
I am working on top of Volker's merging branch to figure issues before they hit me when a beta is released. When the beta, rc or final release arrive I usually have worked out the issues. I have bisected the change of behavior to this ticket. By the sound of it, the change of behavior is accidental and you have no idea or care about it.
On a side note since you are mentioning it, I am not currently using build/pkgs/sagelib/src/setup.py
, links to sources have proved troublesome so far. I am hoping to switch to that at some point when some packaging dust settles. But I should point out that it severely hamper my ability to build the documentation in the same package. Which is a point our current philosophy of building sage are getting divergent. But those are discussions we should have in another space than this ticket.
comment:124 in reply to: ↑ 123 ; followup: ↓ 126 Changed 9 months ago by
comment:125 Changed 9 months ago by
comment:126 in reply to: ↑ 124 Changed 9 months ago by
Replying to mkoeppe:
Replying to fbissey:
By the sound of it, the change of behavior is accidental
No. Since #30779, the Sage distribution has no longer used
src/setup.py
. It switched to usingbuild/pkgs/sagelib/src/setup.py
. You should do the same.
I hadn't realised how much they diverged from each other. It feels like some tickets patch one but not the other and so on. Regardless of my issues and divergence in philosophy, as I called it, if the version in src/setup.py
shouldn't be used or developed against we need to remove it and quickly.
comment:127 Changed 9 months ago by
No, as of this ticket, it's used for ./configure enableeditable
.
comment:128 followup: ↓ 130 Changed 9 months ago by
I think I am starting to get the picture. So those large looking differences are on purpose?
comment:129 Changed 9 months ago by
It sounds like it would be good to add some explanatory comments at the top of src/setup.py
(not by me, since I don't understand the situation).
Branch pushed to git repo; I updated commit sha1. New commits:
build/pkgs/sagelib: Implement enableeditable