fix: make test suite run doctests #31
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The handful of failures on Linux and Mac seem to be due the changed print style in Arb 2.20, so just a matter of updating the doctests. Then for Windows you have a ton of these which probably all result from the same bug somewhere in python-flint (maybe a lingering |
It might actually be a problem in Arb. There are a lot of compiler warnings in the Windows build log from building Arb (before python-flint is built) like: |
|
I've updated the doctests and now the Linux/OSX tests are passing. |
|
Those compiler warnings are false positives in GCC 11 and later; see flintlib/flint#1079 Nevertheless, I suppose to debug this, a good start would be to run the Arb test suite after building Arb, to see if that catches anything? |
Are you sure they're the same? It's |
Yeah, those seems to be two sides of the same coin. Do the same warnings show up when building Flint? |
|
I've attached the part of the Windows build log that builds flint, arb and python-flint: build_log.txt There are some warnings from flint as well like: Building Arb gives many more warnings like that. Building python-flint itself gives one warning about signed/unsigned comparison which isn't seen in the logs I think because cibuildwheel hides it. I see the same warning when building locally on Linux though: $ bin/build_inplace.sh
setup.py:8: DeprecationWarning:
`numpy.distutils` is deprecated since NumPy 1.23.0, as a result
of the deprecation of `distutils` itself. It will be removed for
Python >= 3.12. For older Python versions it will remain present.
It is recommended to use `setuptools < 60.0` for those Python versions.
For more details, see:
https://numpy.org/devdocs/reference/distutils_status_migration.html
from numpy.distutils.system_info import default_include_dirs, default_lib_dirs
running build_ext
cythoning src/flint/pyflint.pyx to src/flint/pyflint.c
/home/oscar/current/sympy/sympy.git/38venv/lib/python3.8/site-packages/Cython/Compiler/Main.py:369: FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release! File: /home/oscar/current/sympy/python-flint/src/flint/_flint.pxd
tree = Parsing.p_module(s, pxd, full_module_name)
building 'flint._flint' extension
INFO: C compiler: x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC
INFO: compile options: '-I/usr/local/include -I/home/oscar/current/sympy/sympy.git/38venv/include -I/usr/local/include/flint -I/home/oscar/current/sympy/sympy.git/38venv/include/flint -I/home/oscar/current/sympy/sympy.git/38venv/include -I/usr/include/python3.8 -c'
INFO: x86_64-linux-gnu-gcc: src/flint/pyflint.c
src/flint/pyflint.c: In function ‘__pyx_pf_5flint_6_flint_14dirichlet_char_4__init__’:
src/flint/pyflint.c:195151:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if (((__pyx_t_5 > __pyx_t_6) != 0)) {
^
src/flint/pyflint.c: At top level:
src/flint/pyflint.c:29067:18: warning: ‘__pyx_f_5flint_6_flint_any_as_fmpz_mpoly’ defined but not used [-Wunused-function]
static PyObject *__pyx_f_5flint_6_flint_any_as_fmpz_mpoly(CYTHON_UNUSED PyObject *__pyx_v_x) {
^
INFO: x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -DNDEBUG -g -fwrapv -O2 -Wall build/temp.linux-x86_64-cpython-38/src/flint/pyflint.o -L/home/oscar/current/sympy/sympy.git/38venv/lib -L/usr/local/lib -L/usr/lib -L/usr/lib/x86_64-linux-gnu -L/usr/lib -larb -lflint -o /home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so |
The full build log can be seen in GitHub Actions and also includes a large number of warnings from compiling GMP about |
|
You could do |
The tests completed quickly enough on Linux and passed. They failed immediately on OSX because it couldn't find the shared library (hopefully fixed by last commit). They failed on Windows: Hopefully that's the same problem as OSX so need to wait for latest commit to see... |
Once this issue is resolved I'll leave the test run in but with a reduced multiplier for ongoing CI. |
There are a lot of places where python-flint uses long but flint uses slong. Should we basically change every use of long in python-flint? |
It would be nice if there was some way to have the types declared in cython actually checked by the C compiler... |
|
Still the same problem with running the tests on Windows. There isn't any meaningful error message so I'm not sure where to go from there. The Arb tests on Linux are passing and it looks like they will pass on OSX as well. |
Yes, this may be necessary. |
The very first test failing suggests that the test programs don't manage to link to the library or something like that. |
|
For now I will remove the |
Probably. That was the problem with OSX. I'd have to switch over to the Windows machine I've been using to try it locally. |
oscarbenjamin
force-pushed
the
pr_doctest
branch
from
1276c4c to
e67a8e0
Compare
|
Okay, that will do for now. This has fixed the doctests and makes sure that they are tested in CI. It currently shows that they fail on Windows but that can be something to fix in another PR. The failing doctests all look like this and are all in The two |
This was referenced Dec 13, 2022
Closed
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently these doctests should fail. I'm going to push them to CI first just to see the output on all platforms and then I'll push another commit to fix the doctests.