#16583 closed enhancement (fixed)
Clean up FLINT declarations
Reported by:  malb  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  basic arithmetic  Keywords:  sd59 
Cc:  Merged in:  
Authors:  Martin Albrecht  Reviewers:  Jeroen Demeyer, Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  20c767c (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Update fmpz.pxi and fmpz_poly.pxi (and others) so that Cython doesn't complain about referenced before assigment any more.
Change History (30)
comment:1 Changed 7 years ago by
 Branch set to u/malb/t16583_fmpz_poly
 Commit set to dfc8ce1e411eee34ce50f69f37fe37c73cbe1f79
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Status changed from needs_review to needs_work
This needs to be rebased, there is a conflict..
comment:3 Changed 7 years ago by
and a typo in the second commit, by the way.
comment:4 Changed 7 years ago by
 Commit changed from dfc8ce1e411eee34ce50f69f37fe37c73cbe1f79 to 8bb6b48617d5e8cdf2a8f50f269332d591dcd8a7
Branch pushed to git repo; I updated commit sha1. New commits:
8bb6b48  merge

comment:5 Changed 7 years ago by
rebased & fixed typo
comment:6 Changed 7 years ago by
Is this related to #13532 ?
comment:7 Changed 7 years ago by
Yep, it removes some of those warnings. The big TODO is to fix mpz_t, though, which is a lot of work.
comment:8 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:9 Changed 6 years ago by
 Branch changed from u/malb/t16583_fmpz_poly to public/ticket/16583
 Commit changed from 8bb6b48617d5e8cdf2a8f50f269332d591dcd8a7 to 14e730e84c2df7b5ccbe4aca490ae8c78d9fa43d
comment:10 Changed 6 years ago by
 Commit changed from 14e730e84c2df7b5ccbe4aca490ae8c78d9fa43d to 18a48429757e11afd32818b8940114e83a511824
Branch pushed to git repo; I updated commit sha1. New commits:
18a4842  trac #16583, oops, correcting wrong merge

comment:11 Changed 6 years ago by
 Status changed from needs_work to needs_review
I guess this should be 'needs review'. Your rebasing looks good.
comment:12 Changed 6 years ago by
In src/sage/libs/flint/fmpz_poly.pxd
, what's the reason you remove the declaration for fmpz_poly_reverse
but not fmpz_poly_revert_series
? I think both can be removed.
comment:13 Changed 6 years ago by
 Branch changed from public/ticket/16583 to u/jdemeyer/ticket/16583
 Created changed from 06/28/14 17:14:18 to 06/28/14 17:14:18
 Modified changed from 08/27/14 20:52:23 to 08/27/14 20:52:23
comment:14 Changed 6 years ago by
 Commit changed from 18a48429757e11afd32818b8940114e83a511824 to c416fee312ff1d8ab43b3a3c650f44688a17c7b9
Looks good. If you agree with my reviewer patch, then it's positive_review for me.
New commits:
c416fee  Remove stuff which is not needed

comment:16 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from positive_review to needs_work
Sorry, just found an issue. In fmpq_poly.pxd
, there still is
ctypedef void * fmpz_t ctypedef void * fmpq_poly_t
I'll create a patch.
comment:17 Changed 6 years ago by
 Commit changed from c416fee312ff1d8ab43b3a3c650f44688a17c7b9 to b3d6aa2c24defa7cd22675f5540659b07a25401c
Branch pushed to git repo; I updated commit sha1. New commits:
b3d6aa2  Further cleanup

comment:18 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:19 Changed 6 years ago by
 Description modified (diff)
 Summary changed from clean up fmpz_t and fmpz_poly_t declarations to Clean up FLINT declarations
comment:21 Changed 6 years ago by
 Status changed from positive_review to needs_work
Some doctests still import fmpq_poly.pxd
:
sage t long src/sage/misc/cython.py ********************************************************************** File "src/sage/misc/cython.py", line 254, in sage.misc.cython.pyx_preparse Failed example: module = sage.misc.cython.import_test("trac11680") # long time (7s on sage.math, 2012) Exception raised: Traceback (most recent call last): File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 480, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 843, in compile_and_execute exec compiled in globs File "<doctest sage.misc.cython.pyx_preparse[9]>", line 1, in <module> module = sage.misc.cython.import_test("trac11680") # long time (7s on sage.math, 2012) File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/misc/cython.py", line 805, in import_test return compile_and_load(TESTS[name]) File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/misc/cython.py", line 757, in compile_and_load return cython_import(file, create_local_c_file=False) File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/server/support.py", line 447, in cython_import **kwds) File "/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/misc/cython.py", line 493, in cython raise RuntimeError("Error converting {} to C:\n{}\n{}".format(filename, log, err)) RuntimeError: Error converting /home/buildslavesage/slave/sage_git/dot_sage/temp/desktop/29631/tmp_i0rQYs.pyx to C: Error compiling Cython file:  ... #cinclude $SAGE_SRC/sage/libs/flint $SAGE_LOCAL/include/FLINT #clib flint from sage.rings.rational cimport Rational from sage.rings.polynomial.polynomial_rational_flint cimport Polynomial_rational_flint from sage.libs.flint.fmpq_poly cimport (fmpq_poly_get_coeff_mpq, fmpq_poly_set_coeff_mpq, ^  _home_buildslave_sage_slave_sage_git_dot_sage_temp_desktop_29631_tmp_i0rQYs_pyx_0.pyx:13:0: 'sage.libs.flint.fmpq_poly.pxd' not found
comment:22 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:23 Changed 6 years ago by
 Commit changed from b3d6aa2c24defa7cd22675f5540659b07a25401c to 20c767c6a446b441b1551a03a10fb09e1e4bb711
Branch pushed to git repo; I updated commit sha1. New commits:
20c767c  Fix include of fmpq_poly.pxi in Cython doctest

comment:24 Changed 6 years ago by
 Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Volker Braun
 Status changed from needs_review to positive_review
comment:25 Changed 6 years ago by
comment:26 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/16583 to 20c767c6a446b441b1551a03a10fb09e1e4bb711
 Resolution set to fixed
 Status changed from positive_review to closed
comment:27 Changed 6 years ago by
 Commit 20c767c6a446b441b1551a03a10fb09e1e4bb711 deleted
Sorry to dig into "old" stuff, but I was suggested to use pxd files rather than pxi files and see the changes here went the other way around... Any strong reason to have done that?
comment:28 Changed 6 years ago by
Generally speaking, Cython got better at what can be cimported over time (pxd). So there is less reason to rely on textual inclusion (pxi). If in doubt (and if it works ;) then use pxd.
comment:29 Changed 6 years ago by
Ok, then I'll take the chance to open a ticket to migrate all of flint stuff to pxd files.
comment:30 Changed 6 years ago by
See #16428.
New commits:
cleanup of fmpz_t and fmpz_poly_t to avoid Cython warnings about referencing before assignment
silence Cython warning about complicated declaration