sage.rings.integer, rational: Remove compile-time dependency on cypari2 and flint

[mkoeppe]

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.

CC: @videlec @kliem @tscrim

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

[videlec]

comment:1

Moving set_from_pari_gen in sage.libs.pari would make cypari2 depends on sage. Wouldn't it?

[mkoeppe]

comment:2

sage.libs.pari.convert_sage already depends on sage (and on cypari2).

[videlec]

comment:3

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.

[mkoeppe]

comment:6

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[mkoeppe]

Branch: u/mkoeppe/sage_rings_integer__rational__remove_compile_time_dependency_on_cypari2_and_flint

[mkoeppe]

Author: Matthias Koeppe, ...

[mkoeppe]

New commits:

08084b3

sage.libs.pari.convert_sage: Move set_integer_from_gen here from sage.rings.integer (set_from_pari_gen)

[mkoeppe]

Commit: 08084b3

[kliem]

New commits:

9986723	remove compile time dependency of Integer and Rational on cypari2
c08a8b5	remove compile-time dependency on flint from integer

[kliem]

Changed commit from 08084b3 to c08a8b5

[kliem]

Changed branch from u/mkoeppe/sage_rings_integer__rational__remove_compile_time_dependency_on_cypari2_and_flint to u/gh-kliem/sage_rings_integer__rational__remove_compile_time_dependency_on_cypari2_and_flint

[kliem]

Changed author from Matthias Koeppe, ... to Matthias Koeppe, Jonathan Kliem

[kliem]

Reviewer: Jonathan Kliem, ...

[mkoeppe]

comment:12

Thanks a lot for working on this!

[mkoeppe]

comment:13

+            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`

[mkoeppe]

comment:14

The next module that will need similar work is sage.rings.fast_arith

[sagetrac-git]

Changed commit from c08a8b5 to 88d91ba

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

88d91ba

use try ... except ... else

[mkoeppe]

comment:16

Also sage.matrix.args has the cypari2 compile-time dep

[mkoeppe]

comment:17

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] 

[sagetrac-git]

Changed commit from 88d91ba to b4477da

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b4477da

fixed indent

[mkoeppe]

comment:19

LGTM, let's get this in.

[mkoeppe]

Changed reviewer from Jonathan Kliem, ... to Jonathan Kliem, Matthias Koeppe

[mkoeppe]

comment:20

Follow-up in #32441

[vbraun]

Changed branch from u/gh-kliem/sage_rings_integer__rational__remove_compile_time_dependency_on_cypari2_and_flint to b4477da