-
Notifications
You must be signed in to change notification settings - Fork 27
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
Bump flint to version 3 #43
Conversation
cd3c157
to
edf6dae
Compare
The extension module fails to compile on OSX:
I saw at least some of those errors but as warnings locally on Linux and the Linux job completes successfully in CI. I guess this is clang being stricter than gcc. |
The functions being complained about are all after the newline here: |
I guess it is because these were moved from |
Okay, I think that might be it. In the previous commit Linux and OSX were working but the Windows build failed at the final repair wheel step because the DLLs for Arb are now placed in Hopefully the last commit will have the Windows build working and then everything here is ready and working with flint 3.0.0-alpha1. It is good to have a branch here with CI passing ready for the flint 3.0.0 release but I don't propose to merge this until flint itself is released. When there is either a new prerelease or a final release of 3.0.0 the version that is pinned here to |
So the Windows build succeeded and the normal tests passed but the doctests have crashed somehow:
I'll see if I can reproduce that locally. |
I can reproduce the doctest crash locally on Windows with the wheel downloaded from CI (we should add $ python ../../test/dtest.py -v
...
Trying:
fmpz(10**35 + 1).factor()
Expecting:
[(11, 1), (9091, 1), (909091, 1), (4147571, 1), (265212793249617641, 1)]
Flint exception (General error):
mkstemp failed |
I guess that error comes from here: |
|
2b671c3
to
cc391ff
Compare
It is expected that the "Build / Install from PyPI sdist on Ubuntu" CI job would fail here. Until a sdist containing this PR is pushed to PyPI the build instructions in the script doing that install will not work because they do not build arb separately and that will not work with the python-flint 0.4.1 sdist that is still on PyPI. The |
It is also expected that the tests will fail on Windows with |
Actually now that CI completes the Windows tests have not failed. I am not sure why they work now when they did not before... The PR still builds with The one failure that is seen (build from PyPI) is still expected though. I just changed the Linux build instructions/demo script in this PR in a way that is not compatible with the current release of python-flint and won't be until a new release is made based on flint-3. |
b91822a
to
07bd650
Compare
07bd650
to
6b3a711
Compare
Rebased after #65 |
@@ -1,6 +1,6 @@ | |||
from flint._flint cimport mp_limb_t, mp_bitcnt_t | |||
|
|||
cdef extern from "flint/nmod_vec.h": | |||
cdef extern from "flint/nmod.h": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deinst I needed to change this for clang (OSX) and Flint 3.
I'm not sure why the problem only shows up with clang and Flint 3. Probably it should changed on master as well: see the last commit in this PR. I don't think it's urgent given that a fix is here ready for Flint 3 when it gets released (I guess that is soon).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My wife's sister and her husband are visiting for a few days so my work will be sporadic, but the new nmod.pxd
will land next week at the latest. I have been updating the flintlib submodule with all the functions from the documentation, and have found a couple of doc bugs. This should be done in the next few days
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to rush!
I doubt there will be a release of python-flint before flint 3 so at least the change above will be merged by then from this PR if not another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the new scraping tool for |
I you would like to fix this PR (which has merge conflicts now) and open a new PR I would be happy to merge it and move to flint3. |
I guess this PR can now be closed? |
Following on from #35 (comment)
Bumps the flint version to 3.0.0-alpha1. Mostly all that is needed is removing
-larb
from setup.py and renaming the Arb headers. Also the build scripts need to not build Arb separately any more.We can use this to see in CI that python_flint is ready for flint 3 but it should only be merged after flint 3 is released.
At first CI will fail because of Cython 3 (gh-35 needs to be merged first or the Cython version should be pinned again).