Auto-generate gen.pyx -- part 1

[jdemeyer]

The files src/sage/libs/pari/gen.pyx and src/sage/libs/pari/pari_instance.pyx can be mostly auto-generated.

This first ticket introduces the infrastructure (except for documentation) and removes the manually-defined functions from gen.pyx and pari_instance.pyx only in trivial cases (function does the same thing and no documentation gets lost).

Follow-up: #17860

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​a18d97f

Auto-generate parts of gen.pyx and pari_instance.pyx

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​58af466

Auto-generate parts of gen.pyx and pari_instance.pyx

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​ade4f75	Upgrade PARI to git master
​fd9074f	Auto-generate parts of gen.pyx and pari_instance.pyx

[vdelecroix]

Is it on purpose that decl.pxi and declinl.pxi not automatically generated as well from 'paridecl.h'? It seems that we even have a parisage.h in between! Is the following in parisage.h still true

#undef coeff  /* Conflicts with NTL */

I made some review commits. You can cherry-pick the one you want:

• commit 1 and 4 are trivial

• Don't we want to keep the order of the pari.desc file? It would be enough to make read_pari_desc an iterator and avoid the sorted in PariFunctionGenerator.__call__. (see commit 3)

• It is not very nice to hardcode the path of pari.desc since some distro might want to build with their own pari. The solution is not perfect but at least the pari source is only hardcoded in one place. (see commit 2)

[jdemeyer]

Where are the review commits?

[vdelecroix]

here they are

[vdelecroix]

New commits:

​ade4f75	Upgrade PARI to git master
​fd9074f	Auto-generate parts of gen.pyx and pari_instance.pyx
​82a67fc	trac #17631 (reviewer commit 1): add a force argument
​8da123a	trac #17631 (reviewer commit 2): pari_src function
​eb5a737	trac #17631 (reviewer commit 3): make read_pari_desc an iterator
​5123666	trac #17631 (reviewer commit 3): simpler read_decl

[jdemeyer]

Replying to vdelecroix:

• Don't we want to keep the order of the pari.desc file? It would be enough to make read_pari_desc an iterator and avoid the sorted in PariFunctionGenerator.__call__. (see commit 3)

I don't think the order really matters. When developing this code, I did find it nice that I could do D["bnfinit"]. So I prefer to not make this change.

I agree (modulo a few details) with the other commits.

[jdemeyer]

New commits:

​b1b29eb	trac #17631 (reviewer commit 3): simpler read_decl
​98022fd	Rename pari_src() to pari_share()

[jdemeyer]

Replying to vdelecroix:

Is it on purpose that decl.pxi and declinl.pxi not automatically generated as well from 'paridecl.h'?

Doing that would be hard. Not impossible, but a highly non-trivial project.

It seems that we even have a parisage.h in between! Is the following in parisage.h still true

#undef coeff  /* Conflicts with NTL */

Yes, we should leave that.

[git]

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

​a5a6b25

Merge tag '6.6.beta4' into t/17631/17631

[vdelecroix]

I am a bit worried since the auto generated files do not get any doctest in them...

What is that in src/sage/libs/pari/pari_instance.pyx?

+            #from sage.misc.superseded import deprecation
+            #deprecation(XXXXX, 'passing -1 as PARI variable is deprecated, use None instead')

Why did you keep so much methods in the gen class in gen.pyx (such as List, Mat, sign, vecmin, vecmax, etc)?

Where can I found the implementation and/or documentation of pari_catch_sig_on and co?

[jdemeyer]

Replying to vdelecroix:

I am a bit worried since the auto generated files do not get any doctest in them...

That's true. On the other hand, I did not remove any doctests either, so it's a least status quo. PARI itself has quite an extensive test suite (run if you install PARI with SAGE_CHECK=yes or with ./sage -i -c pari).

What is that in src/sage/libs/pari/pari_instance.pyx?

+            #from sage.misc.superseded import deprecation
+            #deprecation(XXXXX, 'passing -1 as PARI variable is deprecated, use None instead')

It's basically a reminder to myself that this really should be deprecated.

Why did you keep so much methods in the gen class in gen.pyx (such as List, Mat, sign, vecmin, vecmax, etc)?

In any case, I didn't want to remove any doctests. I also didn't check very carefully which methods could be removed.

Where can I found the implementation and/or documentation of pari_catch_sig_on and co?

See src/sage/libs/pari/pari_err.pxi. But pari_catch_sig_on() and pari_catch_sig_off are just sig_on() and sig_off. This used to be different before we had #14894.

Most of your comments are indeed good comments, but my first priority was to get the general framework (i.e. this ticket) reviewed, we can still improve things later. I prefer to split tickets in smaller pieces, I dislike these huge tickets which do everything.

[vdelecroix]

Replying to jdemeyer:

Replying to vdelecroix:

What is that in src/sage/libs/pari/pari_instance.pyx?

+            #from sage.misc.superseded import deprecation
+            #deprecation(XXXXX, 'passing -1 as PARI variable is deprecated, use None instead')

It's basically a reminder to myself that this really should be deprecated.

The point is that if this ticket gets merged but not the follow-up (because you might stop doing Sage development or anything) we will end up with these two very strange lines.

Could you at least start with a comment

+            # # we should deprecate this because XYZ.
+            #from sage.misc.superseded import deprecation
+            #deprecation(XXXXX, 'passing -1 as PARI variable is deprecated, use None 

Why did you keep so much methods in the gen class in gen.pyx (such as List, Mat, sign, vecmin, vecmax, etc)?

In any case, I didn't want to remove any doctests. I also didn't check very carefully which methods could be removed.

Right. Doctest are indeed important! What is your long term goal with those methods?

Where can I found the implementation and/or documentation of pari_catch_sig_on and co?

See src/sage/libs/pari/pari_err.pxi. But pari_catch_sig_on() and pari_catch_sig_off are just sig_on() and sig_off. This used to be different before we had #14894.

Just for precision: is pari_catch_sig_on just an equivalent to sig_on? If so I really do not understand why there is so much pari_catch_sig_on without any pari_catch_sig_off!

Most of your comments are indeed good comments, but my first priority was to get the general framework (i.e. this ticket) reviewed, we can still improve things later. I prefer to split tickets in smaller pieces, I dislike these huge tickets which do everything.

But I, as a reviewer, do not have your vision of the larger project. I just see one ticket (actually there is also the part 2 #17860).

[jdemeyer]

Replying to vdelecroix:

Why did you keep so much methods in the gen class in gen.pyx (such as List, Mat, sign, vecmin, vecmax, etc)?

In any case, I didn't want to remove any doctests. I also didn't check very carefully which methods could be removed.

Right. Doctest are indeed important! What is your long term goal with those methods?

In the end, I think it's probably best to store all extra documentation and doctests in a separate file (I haven't thought about the exact mechanism). Then that extra documentation will be put together with the PARI documentation in the auto-generated file.

Just for precision: is pari_catch_sig_on just an equivalent to sig_on?

Yes (although you're recommended to use pari_catch_sig_on in PARI code in case we want to change this).

I really do not understand why there is so much pari_catch_sig_on without any pari_catch_sig_off!

Because the new_gen() method runs pari_catch_sig_off().

[jdemeyer]

About vecmin and vecmax: those have a pointer argument (a & in the prototype) which is currently not supported.

[git]

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

​b880274

Remove commented deprecation

[vdelecroix]

Let it go and use it!

[jdemeyer]

Thanks for the review!

[was]

Wow, I'm a little worried about doctests. However, this whole approach is a major step forward! Great work!!

[jdemeyer]

About the doctests: this ticket is about a very direct interface with upstream PARI. That interface itself is doctested.

The wrapped functions indeed lack doctests, but PARI has quite an extensive test suite (which is run by Sage when installing with SAGE_CHECK=yes).