is_prime for ideals uses factorization, can be VERY slow

[yyyyx4]

Fixes #33360.

[yyyyx4]

The single test failure is unrelated and will be fixed by #35017.

[codecov-commenter]

Codecov Report

Base: 88.59% // Head: 88.58% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (bca94c2) compared to base (05329f6).
Patch coverage: 85.71% of modified lines in pull request are covered.

❗ Current head bca94c2 differs from pull request most recent head 3d4d764. Consider uploading reports for the commit 3d4d764 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #34980      +/-   ##
===========================================
- Coverage    88.59%   88.58%   -0.02%     
===========================================
  Files         2140     2140              
  Lines       396998   396963      -35     
===========================================
- Hits        351740   351651      -89     
- Misses       45258    45312      +54     

Impacted Files	Coverage Δ
src/sage/rings/number_field/number_field_ideal.py	93.34% <85.71%> (-0.14%)	⬇️
...e/combinat/cluster_algebra_quiver/mutation_type.py	50.59% <0.00%> (-4.04%)	⬇️
src/sage/modular/local_comp/liftings.py	98.33% <0.00%> (-1.67%)	⬇️
src/sage/schemes/elliptic_curves/hom_velusqrt.py	94.09% <0.00%> (-0.79%)	⬇️
src/sage/groups/generic.py	88.34% <0.00%> (-0.68%)	⬇️
src/sage/schemes/elliptic_curves/hom.py	83.33% <0.00%> (-0.62%)	⬇️
src/sage/matrix/matrix_integer_dense_hnf.py	85.71% <0.00%> (-0.51%)	⬇️
src/sage/quadratic_forms/binary_qf.py	92.78% <0.00%> (-0.50%)	⬇️
src/sage/graphs/tutte_polynomial.py	93.57% <0.00%> (-0.46%)	⬇️
src/sage/combinat/constellation.py	91.18% <0.00%> (-0.41%)	⬇️
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tornaria]

I'm thinking something simple like this:

diff --git a/src/sage/rings/number_field/number_field_ideal.py b/src/sage/rings/number_field/number_field_ideal.py
index 1a9d11aec68..8ca1f958039 100644
--- a/src/sage/rings/number_field/number_field_ideal.py
+++ b/src/sage/rings/number_field/number_field_ideal.py
@@ -1017,13 +1017,15 @@ class NumberFieldIdeal(Ideal_generic):
         K = self.number_field().pari_nf()
         I = self.pari_hnf()
 
-        self._pari_prime = K.idealismaximal(I) or None
+        candidate = K.idealismaximal(I) or None
 
         # PARI uses probabilistic primality testing inside idealismaximal().
-        if self._pari_prime \
-                and get_flag(None, 'arithmetic') \
-                and not self._pari_prime[0].isprime():
-            self._pari_prime = None
+        if get_flag(None, 'arithmetic'):
+            # proof required, check using isprime()
+            if candidate and not candidate[0].isprime():
+                candidate = None
+
+        self._pari_prime = candidate
 
         return self._pari_prime is not None
 

What do you think? You are probably right that there is no way to trigger anything bad here, but it still smells to me. It also feels easier to read if the condition get_flag(None, 'arithmetic') is split and a comment added since that is a pretty opaque name for a function. I couldn't understand what the first parameter is for, and why can't the function be named require_proof used as in require_proof('arithmetic') or require_proof_for('arithmetic') either of which reads more naturally and clearly indicates the meaning.

In any case, do you think it's better to squash everything into a single commit? The final diff is much simpler than 5-6 commits.

[yyyyx4]

Okay, all done, including squashing.

[github-actions]

Documentation preview for this PR is ready! 🎉
Built with commit: 3d4d764

[roed314]

LGTM