sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.39k stars 473 forks source link

add a method to reach lmfdb webpage of elliptic curves over Q #21533

Closed fchapoton closed 7 years ago

fchapoton commented 8 years ago

as suggested in #18968

CC: @JohnCremona @kedlaya @sagetrac-tmonteil

Component: elliptic curves

Author: Frédéric Chapoton

Branch/Commit: 2020242

Reviewer: John Cremona

Issue created by migration from https://trac.sagemath.org/ticket/21533

fchapoton commented 8 years ago

New commits:

d4781c3trying to add links to lmfdb (not working)
8d7831eMerge branch 'public/18968' in 7.4.b5
2020242trac 18968 link to lmfdb page with correct url
fchapoton commented 8 years ago

Commit: 2020242

fchapoton commented 8 years ago

Branch: u/chapoton/21533

fchapoton commented 8 years ago
comment:3

any opinion on this proposal ?

JohnCremona commented 8 years ago
comment:4

Is there anywhere else in Sage where a function opens a web page ? Just wondering.

fchapoton commented 8 years ago
comment:5

There are already two instances:

git grep -P -c "webbrowser.open"
src/sage/databases/findstat.py:5
src/sage/databases/oeis.py:5
fchapoton commented 7 years ago
comment:6

ping ?

JohnCremona commented 7 years ago
comment:8

I will test this. It should be tested with and without the optional large database -- my own installations probably all have it so I hope someone else can test without.

JohnCremona commented 7 years ago
comment:9

I tested the branch after merging into current develop (7.6.beta0). It worked fine, starting a browser the first time and then opening new tabs in the same browser after that. If the curve is not in the database it gives a reasonable run-time error.

I'm sure that people will say that they should not have to install the optional database of curves if they can get the data from the LMFDB website; true (unless running Sage off-line) but work for the future.

Should we allow people to open the LMFDB URL of a valid elliptic curve label without constructin the curve? There could be a stand-alone function called (something like) open_lmfdb_elliptic_curve_page('label'), and then the elliptic curve method could call that.

Another follow-up would be for curves over number fields, which are currently being added to the database. But thelabelling is rather complicated to explain in this margin...

fchapoton commented 7 years ago
comment:10

I am not favourable to allow this call without building the curve.

JohnCremona commented 7 years ago
comment:11

That's OK. I'll just finish testing then and expect to give a +ve review soon.

JohnCremona commented 7 years ago
comment:12

When I remembered to doctest run the file with --optional=webbrowser,sage it worked fine (causing a page to pop up).

I did not test the part where an LMFDB label is used instead, since the curves obtained from the database do not have this field set. There's another ticket somewhere which will add these, and when that happens we'll see if that part of this code works as it should.

JohnCremona commented 7 years ago

Reviewer: John Cremona

vbraun commented 7 years ago

Changed branch from u/chapoton/21533 to 2020242