#9330 closed defect (fixed)
Documentation for sha_tate.py not quite looking right
Reported by: | Karl-Dieter Crisman | Owned by: | John Cremona |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.6 |
Component: | elliptic curves | Keywords: | |
Cc: | John Palmieri | Merged in: | sage-4.6.alpha1 |
Authors: | Chris Wuthrich | Reviewers: | Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
When you look at this, there are a number of things wrong or confusing in the documentation.
Most importantly, several instances of Sha should have ticks, probably. But are they referring to the mathematical object
`Sha`
or the computer structure of the class
``Sha``
? If I knew what was intended (given that the distinction is quite small), I would do this patch myself. But it looks like sometimes the group is intended, other times the class object.
In line 756 we have
We get no information the curve has rank 2.::
Attachments (2)
Change History (17)
comment:1 Changed 12 years ago by
Cc: | jpalmieri added |
---|
comment:2 Changed 12 years ago by
Does jsmath support this? Otherwise one would have to do the right encoding. Anyway, it looks like my job here is done - interest has been piqued ;)
comment:3 Changed 12 years ago by
Cc: | John Palmieri added; jpalmieri removed |
---|
See #9442 for the warning when building the reference manual.
comment:4 Changed 12 years ago by
Authors: | → Wuthrich |
---|---|
Status: | new → needs_review |
I changed Sha to Sha
thoughout the document as it should be. There should be a better solution for this. Alas the unicode letter Ш does not work in the pdf version (it works fine in the html). The \textcyr does not seem to work with LaTeX either. Probably it needs addtional fonts or so and I would object to include them just because of this one letter.
So I think this is as far as I can do the changes.
I also renamed constistantly the group as Tate-Shafarevich and not Shafarevich-Tate. (There is a debate about this, which I do not want to see repeated here. google tells me that T-S is more frequent.)
comment:5 Changed 12 years ago by
Authors: | Wuthrich → Chris Wuthrich |
---|---|
Description: | modified (diff) |
Reviewers: | → Karl-Dieter Crisman |
Status: | needs_review → needs_work |
There are a few other things in the Description which need to be taken care of; in the first case, it's to add double ticks, and in the second it's (probably) to add the word 'when'.
I removed the issue from #9442 from the Description since that is already merged in rc0 - one will probably have to rebase (very slightly) against 4.5.2.rc0 or 4.5.2, since that has been merged. Once that's done (and once I build one of those) I'll also check whether it looks right, but from a cursory glance this looks like a great improvement (and in consistency!). Or jhpalmieri can do it :)
comment:6 follow-up: 9 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
I don't understand this. My file sha_tate.py looks well different from the documentation link that you give. For instance I have changed the descent... thingy in trac 9287 merged as 14603 in 4.5.2.alpha1.
It should always be Sha
never Sha
.
I believe this ticket solves the remaining problems. Though I have not checked it is needs to be rebased.
comment:7 Changed 12 years ago by
Oops the ` went away. I meant
It should always be `Sha`
never ``Sha``
comment:8 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:9 Changed 12 years ago by
Replying to wuthrich:
I don't understand this. My file sha_tate.py looks well different from the documentation link that you give. For instance I have changed the
descent... thingy in trac 9287 merged as 14603 in 4.5.2.alpha1.
Yes, sorry I didn't see that - I've changed the description. What is up with the "We get no information the " bit? Maybe this is grammatically correct in the context of Tate-Shaf groups?
Incidentally, though you changed the references to T-S, the file is S-T :) Oh well; maybe that's as it should be if there isn't agreement.
It should always be
Sha
neverSha
.
I believe this ticket solves the remaining problems. Though I have not checked it is needs to be rebased.
Unfortunately I can't check this right now either, but if someone else doesn't, I'll do so soon.
Changed 12 years ago by
Attachment: | trac_9330_2.patch added |
---|
exported against 4.5.2.alpha1, to be apply after trac_9330.patch
comment:10 Changed 12 years ago by
Yop, sorry, I did not see that the 'when' was still missing. Here is an additional patch.
Luckily the filename is not visible to the user, so I don't have to bother changing it. ... and it gives some voice to the other order.
There is no problem with the rebase, because the patches here are exported against 4.5.2.alpha1 and #9442 was merged in 4.5.alpha4 if I am not mistaken. At least the addition lines are clearly in the file before I edited it.
comment:11 Changed 12 years ago by
Unfortunately, I can't get this to work right when doing sage-docbuild reference html
on my computer (probably due to latex not being in my PATH). Someone else will have to review it, though it seems good!
comment:12 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
I figured out how to get my latex in my PATH finally. There are a few things which *could* be put in ticks or double ticks still, but not ones I feel are necessary at this point, more ambiguous - and none related to Sha. Positive review
comment:13 Changed 12 years ago by
Merged in: | → sage-4.6.alpha1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:14 follow-up: 15 Changed 12 years ago by
These changes also require modifying doctests in sage/schemes/elliptic_curves/BSD.py
(Shafarevich-Tate -> Tate-Shafarevich).
I don't know if there's a separate ticket for this.
Is there any reason we cannot use the proper cyrillic font for Sha?
\newcommand{\Sha}{{\mbox{\textcyr{Sh}}}}