sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

PARI/GP error on attempt to call .n() on GpElement #33574

Open maxale opened 2 years ago

maxale commented 2 years ago

gp.content([1,2,3]).n() produces an error like

TypeError: Error executing code in GP:
CODE:
    sage[11]=prec(sage[10]);
PARI/GP ERROR:
  ***   at top-level: sage[11]=prec(sage[10])
  ***                          ^--------------
  ***   not a function in function call
A function with that name existed in GP-1.39.15. Please update your script.

New syntax: prec(x,n) ===> precision(x,n)

precision(x,{n}): if n is present, return x at precision n. If n is omitted, 
return real precision of object x.

Component: numerical

Branch/Commit: u/gh-maxale/gp_change_prec_to_precision @ 7f37a1f

Issue created by migration from https://trac.sagemath.org/ticket/33574

maxale commented 2 years ago
comment:2

It seems the issue is fixed with the following simple change:

--- numerical_approx.pyx.OLD    2022-06-08 15:46:38.799629463 -0400
+++ numerical_approx.pyx    2022-06-08 15:46:50.847671116 -0400
@@ -52,7 +52,7 @@

     # Figure out input precision to check for case (1)
     try:
-        inprec = x.prec()
+        inprec = x.precision()
     except AttributeError:
         if prec > 53 and CDF.has_coerce_map_from(P):
             # If we can coerce to CDF, assume input precision was 53 bits

I'd like to propose it as a fix for this ticket, but I'm not sure how.

fchapoton commented 2 years ago
comment:3

do you know git ?

if yes, you just need to push a branch here (after cloning and setting the trac remote)

git push trac HEAD:u/gh-maxale/branchname_at_choice

and then copy this branch name in the ticket field "Branch:"

official instructions are there: https://doc.sagemath.org/html/en/developer/manual_git.html

maxale commented 2 years ago

Commit: 7f37a1f

maxale commented 2 years ago

Branch: u/gh-maxale/gp_change_prec_to_precision

maxale commented 2 years ago

New commits:

7f37a1fUse GP function precision() instead of outdated prec()
fchapoton commented 2 years ago
comment:5

good. Please add a doctest showing that something previously broken now does work.

maxale commented 2 years ago
comment:6

I've never done that before. Are there instructions on how to create a doctest?

fchapoton commented 2 years ago
comment:7

The tests are part of the documentation.

This means you just have to add a few lines in the EXAMPLES or TESTS section of the function that you are changing.

in the present case, I suggest to add a TESTS:: section below the existing EXAMPLES section : fill with example written in the ticket description.

Use the existing documention as a model, with care about the correct indentation.

some instructions here: https://doc.sagemath.org/html/en/developer/coding_basics.html#writing-testable-examples

fchapoton commented 2 years ago
comment:9

still needs that you add a doctest

maxale commented 2 months ago

On a second thought, it looks that requesting a doctest for fixing a code artifact is an overkill. It does not introduce any new features, fixes just a single line of code, and clearly follows the instruction from the underlying PARI library about the change in syntax: New syntax: prec(x,n) ===> precision(x,n)