Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#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:

Status badges

Description (last modified by Karl-Dieter Crisman)

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)

trac_9330.patch (12.7 KB) - added by wuthrich 12 years ago.
exported against 4.5.2.alpha1
trac_9330_2.patch (777 bytes) - added by wuthrich 12 years ago.
exported against 4.5.2.alpha1, to be apply after trac_9330.patch

Download all attachments as: .zip

Change History (17)

comment:1 Changed 12 years ago by John Cremona

Cc: jpalmieri added

Is there any reason we cannot use the proper cyrillic font for Sha?

\newcommand{\Sha}{{\mbox{\textcyr{Sh}}}}

comment:2 Changed 12 years ago by Karl-Dieter Crisman

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 John Palmieri

Cc: John Palmieri added; jpalmieri removed

See #9442 for the warning when building the reference manual.

Changed 12 years ago by wuthrich

Attachment: trac_9330.patch added

exported against 4.5.2.alpha1

comment:4 Changed 12 years ago by wuthrich

Authors: Wuthrich
Status: newneeds_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 Karl-Dieter Crisman

Authors: WuthrichChris Wuthrich
Description: modified (diff)
Reviewers: Karl-Dieter Crisman
Status: needs_reviewneeds_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 Changed 12 years ago by wuthrich

Status: needs_workneeds_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 wuthrich

Oops the ` went away. I meant

It should always be `Sha` never ``Sha``

comment:8 Changed 12 years ago by Karl-Dieter Crisman

Description: modified (diff)

comment:9 in reply to:  6 Changed 12 years ago by Karl-Dieter Crisman

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 never Sha.

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 wuthrich

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 wuthrich

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 Karl-Dieter Crisman

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 Karl-Dieter Crisman

Status: needs_reviewpositive_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 Mitesh Patel

Merged in: sage-4.6.alpha1
Resolution: fixed
Status: positive_reviewclosed

comment:14 Changed 12 years ago by Leif Leonhardy

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.

comment:15 in reply to:  14 Changed 12 years ago by Leif Leonhardy

Replying to leif:

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.

I've opened #9916 for that. Patch needs review.

Note: See TracTickets for help on using tickets.