sage.rings.integer, rational: Remove compile-time dependency on cypari2 and flint
Closed this issue · 27 comments
Functions such as set_from_pari_gen should probably be moved to sage.libs.pari.convert_sage.
The compile-time dependency on sage.libs.flint.ulong_extras is for the use of n_factor.
Component: refactoring
Author: Matthias Koeppe, Jonathan Kliem
Branch/Commit: b4477da
Reviewer: Jonathan Kliem, Matthias Koeppe
Issue created by migration from https://trac.sagemath.org/ticket/30022
Moving set_from_pari_gen in sage.libs.pari would make cypari2 depends on sage. Wouldn't it?
sage.libs.pari.convert_sage already depends on sage (and on cypari2).
I misunderstood the ticket description... I thought you wanted to move that to cypari2. Sorry.
It indeed makes sense to have it in sage.libs where all the low-level interfaces actually are.
Description changed:
---
+++
@@ -1,4 +1,4 @@
-Functions such as `set_from_pari_gen` should probably be moved to `sage.libs.pari`.
+Functions such as `set_from_pari_gen` should probably be moved to `sage.libs.pari.convert_sage`.
The compile-time dependency on `sage.libs.flint.ulong_extras` is for the use of `n_factor`.
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
Author: Matthias Koeppe, ...
New commits:
08084b3 | sage.libs.pari.convert_sage: Move set_integer_from_gen here from sage.rings.integer (set_from_pari_gen) |
Changed author from Matthias Koeppe, ... to Matthias Koeppe, Jonathan Kliem
Reviewer: Jonathan Kliem, ...
Thanks a lot for working on this!
+ try:
+ from sage.libs.flint.ulong_extras import n_factor_to_list
+ if proof is None:
+ from sage.structure.proof.proof import get_flag
+ proof = get_flag(proof, "arithmetic")
+ F = n_factor_to_list(mpz_get_ui(n.value), proof)
+ F = [(smallInteger(a), smallInteger(b)) for a, b in F]
+ F.sort()
+ return IntegerFactorization(F, unit=unit, unsafe=True,
+ sort=False, simplify=False)
+ except ImportError:
+ pass
I think it's better to use try: except: else here, only wrapping the actual import in try-except`
The next module that will need similar work is sage.rings.fast_arith
Branch pushed to git repo; I updated commit sha1. New commits:
88d91ba | use try ... except ... else |
Also sage.matrix.args has the cypari2 compile-time dep
patchbot indicates docbuild failure
[dochtml] cd /home/sagemath/sage-9.1 && ./sage --docbuild --no-pdf-links reference/matrices inventory --no-prune-empty-dirs
[dochtml] [libs ] docstring of sage.libs.pari.convert_sage.pari_divisors_small:3: WARNING: Content block expected for the "SEEALSO" directive; none found.
[dochtml] [libs ] The inventory files are in local/share/doc/sage/inventory/en/reference/libs.
[dochtml] Error building the documentation.
[dochtml] Traceback (most recent call last):
[dochtml] File "/usr/lib/python3.7/runpy.py", line 193, in _run_module_as_main
[dochtml] "__main__", mod_spec)
[dochtml] File "/usr/lib/python3.7/runpy.py", line 85, in _run_code
[dochtml] exec(code, run_globals)
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/__main__.py", line 2, in <module>
[dochtml] main()
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/__init__.py", line 1813, in main
[dochtml] builder()
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/__init__.py", line 821, in _wrapper
[dochtml] getattr(DocBuilder, build_type)(self, *args, **kwds)
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/__init__.py", line 133, in f
[dochtml] runsphinx()
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/sphinxbuild.py", line 323, in runsphinx
[dochtml] sys.stderr.raise_errors()
[dochtml] File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage_docbuild/sphinxbuild.py", line 258, in raise_errors
[dochtml] raise OSError(self._error)
[dochtml] OSError: docstring of sage.libs.pari.convert_sage.pari_divisors_small:3: WARNING: Content block expected for the "SEEALSO" directive; none found.
[dochtml]
Branch pushed to git repo; I updated commit sha1. New commits:
b4477da | fixed indent |
LGTM, let's get this in.
Changed reviewer from Jonathan Kliem, ... to Jonathan Kliem, Matthias Koeppe