Opened 7 years ago
Closed 7 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, GitHub, GitLab) | Commit: | bc1497d6bd0d2e7be42cb85a0596bb1d109438b4 |
Dependencies: | #17631, #17906 | Stopgaps: |
Description
Add documentation from PARI to the auto-generated functions.
Change History (36)
comment:1 Changed 7 years ago by
- Dependencies set to #17631
comment:2 Changed 7 years ago by
- Cc vdelecroix added
comment:3 Changed 7 years ago by
- Branch set to u/jdemeyer/ticket/17860
comment:4 Changed 7 years ago by
- Commit set to 26b3aa1ab78bb1582929581db558f8abb2cc9c77
comment:5 Changed 7 years ago by
- Dependencies changed from #17631 to #17631, #17906
comment:6 Changed 7 years ago by
- Commit changed from 26b3aa1ab78bb1582929581db558f8abb2cc9c77 to 22a5ffd20dc12e21c1cb4a3a6922f59f461fed9a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a5a6b25 | Merge tag '6.6.beta4' into t/17631/17631
|
b880274 | Remove commented deprecation
|
7d1b5f8 | Upgrade PARI to latest master
|
c98e78d | Merge commit '7d1b5f8ca56180ca2d7044453707c619ef17b51a' into HEAD
|
22a5ffd | Add PARI documentation
|
comment:7 Changed 7 years ago by
- Commit changed from 22a5ffd20dc12e21c1cb4a3a6922f59f461fed9a to c683c1946e0bb790996664b06b989a88ae50ce18
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c683c19 | Add PARI documentation
|
comment:8 Changed 7 years ago by
- Status changed from new to needs_review
comment:9 follow-up: ↓ 11 Changed 7 years ago by
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: ↓ 12 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
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 7 years ago by
It looks globally nice!
There are some bad formatting:
- some
:emphasis:
gets wrong (seebnrconductor
orbnrdisc
) - some
:math:
directives get wrong (seebinomial
,contfrac
,divrem
orelleta
). Apparently, some backslash disappeared since we have:math:`binomial(x,y)`
instead of:math:`\binomial{x}{y}`
. Idem inpolgalois
:TbelongstoQQ[X]
- the hyperlink to sections via
[Label: se:XYZ]
are very bad (seefactornf
,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: ↓ 15 Changed 7 years ago by
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 7 years ago by
Replying to jdemeyer:
I can fix the
[Label]
stuff but the rest will require changing the whole approach (not usinggphelp -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 7 years ago by
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 7 years ago by
- Commit changed from c683c1946e0bb790996664b06b989a88ae50ce18 to 4b071619a00e667d3452a73229de43f231dfe662
Branch pushed to git repo; I updated commit sha1. New commits:
4b07161 | Better format links
|
comment:18 Changed 7 years ago by
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 7 years ago by
NOTE: despite the red link to the branch in the Trac interface, the branch merges cleanly.
comment:20 Changed 7 years ago by
- 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 7 years ago by
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 7 years ago by
- Status changed from positive_review to needs_work
comment:23 follow-up: ↓ 24 Changed 7 years ago by
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 7 years ago by
comment:25 Changed 7 years ago by
At least for now you could change the doctest to print get_rest_doc("teichmuller").encode('ascii', errors='ignore')
comment:26 Changed 7 years ago by
- Commit changed from 4b071619a00e667d3452a73229de43f231dfe662 to bc1497d6bd0d2e7be42cb85a0596bb1d109438b4
Branch pushed to git repo; I updated commit sha1. New commits:
bc1497d | Always run doctests with UTF-8 encoding
|
comment:27 Changed 7 years ago by
Does this commit help?
comment:28 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:29 follow-up: ↓ 31 Changed 7 years ago by
Works but I would then prefer to always set it in sage-env
instead of only when doctesting
comment:31 in reply to: ↑ 29 Changed 7 years ago by
- 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: ↓ 33 Changed 7 years ago by
- 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 7 years ago by
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: ↓ 35 Changed 7 years ago by
Could the new features introduced in this ticket be used to fix #1062?
comment:35 in reply to: ↑ 34 Changed 7 years ago by
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 7 years ago by
- Branch changed from u/jdemeyer/ticket/17860 to bc1497d6bd0d2e7be42cb85a0596bb1d109438b4
- Resolution set to fixed
- Status changed from positive_review to closed
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:
Upgrade PARI to git master
Auto-generate parts of gen.pyx and pari_instance.pyx
trac #17631 (reviewer commit 1): add a force argument
trac #17631 (reviewer commit 2): pari_src function
trac #17631 (reviewer commit 3): simpler read_decl
Rename pari_src() to pari_share()
Add PARI documentation