Opened 4 years ago
Closed 3 years ago
#21533 closed enhancement (fixed)
add a method to reach lmfdb webpage of elliptic curves over Q
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.6 |
Component: | elliptic curves | Keywords: | |
Cc: | cremona, kedlaya, tmonteil | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | 2020242 (Commits) | Commit: | 20202426570dd3b613d0ab3c56bf5017337729db |
Dependencies: | Stopgaps: |
Description
as suggested in #18968
Change History (13)
comment:1 Changed 4 years ago by
- Branch set to u/chapoton/21533
- Cc cremona added
- Commit set to 20202426570dd3b613d0ab3c56bf5017337729db
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Cc kedlaya added
comment:3 Changed 4 years ago by
any opinion on this proposal ?
comment:4 Changed 4 years ago by
Is there anywhere else in Sage where a function opens a web page ? Just wondering.
comment:5 Changed 4 years ago by
There are already two instances:
git grep -P -c "webbrowser.open" src/sage/databases/findstat.py:5 src/sage/databases/oeis.py:5
comment:7 Changed 3 years ago by
- Milestone changed from sage-7.4 to sage-7.6
comment:8 Changed 3 years ago by
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.
comment:9 Changed 3 years ago by
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...
comment:10 Changed 3 years ago by
I am not favourable to allow this call without building the curve.
comment:11 Changed 3 years ago by
That's OK. I'll just finish testing then and expect to give a +ve review soon.
comment:12 Changed 3 years ago by
- Reviewers set to John Cremona
- Status changed from needs_review to positive_review
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.
comment:13 Changed 3 years ago by
- Branch changed from u/chapoton/21533 to 20202426570dd3b613d0ab3c56bf5017337729db
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
trying to add links to lmfdb (not working)
Merge branch 'public/18968' in 7.4.b5
trac 18968 link to lmfdb page with correct url