Opened 5 years ago

Closed 4 years ago

#17860 closed enhancement (fixed)

Auto-generate gen.pyx -- part 2

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.6
Component: interfaces Keywords:
Cc: vdelecroix Merged in:
Authors: Jeroen Demeyer Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: bc1497d (Commits) Commit: bc1497d6bd0d2e7be42cb85a0596bb1d109438b4
Dependencies: #17631, #17906 Stopgaps:

Description

Add documentation from PARI to the auto-generated functions.

Change History (36)

comment:1 Changed 5 years ago by jdemeyer

  • Dependencies set to #17631

comment:2 Changed 5 years ago by jdemeyer

  • Cc vdelecroix added

comment:3 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17860

comment:4 Changed 5 years ago by jdemeyer

  • Commit set to 26b3aa1ab78bb1582929581db558f8abb2cc9c77

This is 95% finished, there are still some loose ends but I'd like to wait at least for #16997 to be merged before continuing.


New commits:

ade4f75Upgrade PARI to git master
fd9074fAuto-generate parts of gen.pyx and pari_instance.pyx
82a67fctrac #17631 (reviewer commit 1): add a force argument
8da123atrac #17631 (reviewer commit 2): pari_src function
b1b29ebtrac #17631 (reviewer commit 3): simpler read_decl
98022fdRename pari_src() to pari_share()
26b3aa1Add PARI documentation

comment:5 Changed 5 years ago by jdemeyer

  • Dependencies changed from #17631 to #17631, #17906

comment:6 Changed 5 years ago by git

  • Commit changed from 26b3aa1ab78bb1582929581db558f8abb2cc9c77 to 22a5ffd20dc12e21c1cb4a3a6922f59f461fed9a

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

a5a6b25Merge tag '6.6.beta4' into t/17631/17631
b880274Remove commented deprecation
7d1b5f8Upgrade PARI to latest master
c98e78dMerge commit '7d1b5f8ca56180ca2d7044453707c619ef17b51a' into HEAD
22a5ffdAdd PARI documentation

comment:7 Changed 5 years ago by git

  • Commit changed from 22a5ffd20dc12e21c1cb4a3a6922f59f461fed9a to c683c1946e0bb790996664b06b989a88ae50ce18

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

c683c19Add PARI documentation

comment:8 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:9 follow-up: Changed 5 years ago by vdelecroix

One issue: the documentation is not generated by default from a simple make. I had to do

sage: from sage_setup.autogen import autogen_all
sage: autogen_all(force=True)

comment:10 follow-up: Changed 5 years ago by vdelecroix

Another one: the file auto_gen.pxi does not appear in the reference manual. The documentation is hence invisible (except the interactive one).

comment:11 in reply to: ↑ 9 Changed 4 years ago by jdemeyer

Replying to vdelecroix:

One issue: the documentation is not generated by default from a simple make.

Unfortunately, there is no easy solution for this. If you really think that this should be fixed, we should probably use a Makefile instead of a Python script.

comment:12 in reply to: ↑ 10 Changed 4 years ago by jdemeyer

Replying to vdelecroix:

Another one: the file auto_gen.pxi does not appear in the reference manual. The documentation is hence invisible (except the interactive one).

It's included in gen.pyx, also with the documentation.

comment:13 Changed 4 years ago by vdelecroix

It looks globally nice!

There are some bad formatting:

  • some :emphasis: gets wrong (see bnrconductor or bnrdisc)
  • some :math: directives get wrong (see binomial, contfrac, divrem or elleta). Apparently, some backslash disappeared since we have :math:`binomial(x,y)` instead of :math:`\binomial{x}{y}`. Idem in polgalois: TbelongstoQQ[X]
  • the hyperlink to sections via [Label: se:XYZ] are very bad (see factornf, fflog, fforder, galoisfixedfield, znlog, znorder)

Some of the issue above looks fine in the pari.desc file... might be because of the gphelp script. For example

$ gphelp -raw binomial
...
binomial coefficient @[dollar]binom{x}{y}@[dollar].
...

$ gphelp -raw polgalois
...
@[dollar]T belongs to @[startbi]Q@[endbi][X]@[dollar]
...

On the other hand, the tex documentation with gphelp XYZ looks fine.

comment:14 follow-up: Changed 4 years ago by jdemeyer

I can fix the [Label] stuff but the rest will require changing the whole approach (not using gphelp -raw). What do you think I should do...?

comment:15 in reply to: ↑ 14 Changed 4 years ago by vdelecroix

Replying to jdemeyer:

I can fix the [Label] stuff but the rest will require changing the whole approach (not using gphelp -raw). What do you think I should do...?

I think that it is already a lot of stuff and not so bad as a first approximation. Do you have an idea to get the right documentation everywhere? Do you understand the part of the gphelp script that generates the raw documentation?

comment:16 Changed 4 years ago by jdemeyer

I'm not sure that it's a good idea to hack on the gphelp script. It's probably better (but scary) to parse the TeX code manually.

comment:17 Changed 4 years ago by git

  • Commit changed from c683c1946e0bb790996664b06b989a88ae50ce18 to 4b071619a00e667d3452a73229de43f231dfe662

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

4b07161Better format links

comment:18 Changed 4 years ago by jdemeyer

I fixed the [Label] formatting.

I also manually changed the version of the PARI package, which will cause the auto-generated functions to be regenerated.

comment:19 Changed 4 years ago by jdemeyer

NOTE: despite the red link to the branch in the Trac interface, the branch merges cleanly.

comment:20 Changed 4 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

Some almost clean documentation is much better than no documentation... I will try to see next week with Bill for implementing more options in the gphelp script.

Vincent

comment:21 Changed 4 years ago by vbraun

On OSX I'm getting:

sage -t --long src/sage_setup/autogen/pari/doc.py
**********************************************************************
File "src/sage_setup/autogen/pari/doc.py", line 254, in sage_setup.autogen.pari.doc.get_rest_doc
Failed example:
    print get_rest_doc("teichmuller")
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_setup.autogen.pari.doc.get_rest_doc[1]>", line 1, in <module>
        print get_rest_doc("teichmuller")
    UnicodeEncodeError: 'ascii' codec can't encode character u'\xfc' in position 6: ordinal not in range(128)
**********************************************************************
File "src/sage_setup/autogen/pari/doc.py", line 288, in sage_setup.autogen.pari.doc.get_rest_doc
Failed example:
    print get_rest_doc("ellap")
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_setup.autogen.pari.doc.get_rest_doc[3]>", line 1, in <module>
        print get_rest_doc("ellap")
    UnicodeEncodeError: 'ascii' codec can't encode character u'\xa0' in position 1989: ordinal not in range(128)
**********************************************************************
File "src/sage_setup/autogen/pari/doc.py", line 339, in sage_setup.autogen.pari.doc.get_rest_doc
Failed example:
    print get_rest_doc("bitor")
Exception raised:
    Traceback (most recent call last):
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_setup.autogen.pari.doc.get_rest_doc[4]>", line 1, in <module>
        print get_rest_doc("bitor")
    UnicodeEncodeError: 'ascii' codec can't encode character u'\xa0' in position 125: ordinal not in range(128)
**********************************************************************
1 item had failures:
   3 of   6 in sage_setup.autogen.pari.doc.get_rest_doc
    [14 tests, 3 failures, 0.25 s]

The # coding=utf-8 at the top looks suspicious, is this actually legal?

comment:22 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

comment:23 follow-up: Changed 4 years ago by vbraun

Slightly different on ARM:

sage -t --long src/sage_setup/autogen/pari/doc.py
**********************************************************************
File "src/sage_setup/autogen/pari/doc.py", line 254, in sage_setup.autogen.pari.doc.get_rest_doc
Failed example:
    print get_rest_doc("teichmuller")
Expected:
    Teichmüller character of the :math:`p`-adic number :math:`x`, i.e. the unique
    :math:`(p-1)`-th root of unity congruent to :math:`x / p^{v_p(x)}` modulo :math:`p`.
Got:
    Teichm�ller character of the :math:`p`-adic number :math:`x`, i.e. the unique
    :math:`(p-1)`-th root of unity congruent to :math:`x / p^{v_p(x)}` modulo :math:`p`.
**********************************************************************

The fun is presumably that sys.stdout.encoding in the doctesting framework need not be utf-8

comment:24 in reply to: ↑ 23 Changed 4 years ago by jdemeyer

Replying to vbraun:

The fun is presumably that sys.stdout.encoding in the doctesting framework need not be utf-8

Indeed, see #14153.

comment:25 Changed 4 years ago by vbraun

At least for now you could change the doctest to print get_rest_doc("teichmuller").encode('ascii', errors='ignore')

comment:26 Changed 4 years ago by git

  • Commit changed from 4b071619a00e667d3452a73229de43f231dfe662 to bc1497d6bd0d2e7be42cb85a0596bb1d109438b4

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

bc1497dAlways run doctests with UTF-8 encoding

comment:27 Changed 4 years ago by jdemeyer

Does this commit help?

comment:28 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:29 follow-up: Changed 4 years ago by vbraun

Works but I would then prefer to always set it in sage-env instead of only when doctesting

comment:30 Changed 4 years ago by vbraun

  • Status changed from needs_review to needs_work

Thoughts?

comment:31 in reply to: ↑ 29 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to vbraun:

Works but I would then prefer to always set it in sage-env instead of only when doctesting

That would be a very bad idea. We shouldn't interfere with encodings when doing "normal" Sage or Python work.

comment:32 follow-up: Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

But we should also have doctests behave like the interactive Sage session. If we can't paste an interactive example into a doctests then we are broken... But then we can always see how many people trip over this.

comment:33 in reply to: ↑ 32 Changed 4 years ago by jdemeyer

Replying to vbraun:

But we should also have doctests behave like the interactive Sage session. If we can't paste an interactive example into a doctests then we are broken...

That's a good goal which we should try to approximate as best as possible, but it will never be 100% possible. I consider terminal encodings to be a operating system-specific issue, so outside the scope of the Sage doctesting framework.

comment:34 follow-up: Changed 4 years ago by mmezzarobba

Could the new features introduced in this ticket be used to fix #1062?

comment:35 in reply to: ↑ 34 Changed 4 years ago by jdemeyer

Replying to mmezzarobba:

Could the new features introduced in this ticket be used to fix #1062?

Probably yes. But then you would need to move the code for generating docs from sage_setup to sage.

comment:36 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/17860 to bc1497d6bd0d2e7be42cb85a0596bb1d109438b4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.