Opened 2 years ago
Closed 2 years ago
#25665 closed defect (fixed)
Don't use installed_packages() for threejs URL
Reported by:  egourgoulhon  Owned by:  

Priority:  blocker  Milestone:  sage8.3 
Component:  graphics  Keywords:  threejs 
Cc:  embray, jdemeyer, arojas, fbissey  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  867e1ff (Commits)  Commit:  867e1ffe5b15a3827d2828ad57ac23f392f7f4ee 
Dependencies:  Stopgaps: 
Description (last modified by )
Since Sage 8.3.beta3, running the command
show(sphere(), viewer='threejs', online=True)
in a Jupyter notebook displays nothing. It was fine in Sage <= 8.3.beta2.
Note that the option online=True
is required by tools like http://nbviewer.jupyter.org/ and CoCalc.
It seems that the issue is due to the following change introduced in #25040 (which has been merged in Sage 8.3.beta3):
sage: installed_packages()['threejs'] 'r80.p0'
while in Sage <= 8.3.beta2, one has
sage: installed_packages()['threejs'] 'r80'
Indeed the value of installed_packages()['threejs']
is used to form the url in lines 749750 of src/sage/repl/rich_output/display_manager.py
Change History (13)
comment:1 Changed 2 years ago by
 Cc embray jdemeyer added
comment:2 in reply to: ↑ description ; followup: ↓ 5 Changed 2 years ago by
comment:3 Changed 2 years ago by
 Description modified (diff)
 Summary changed from threejs viewer broken after Sage 8.3.beta3 to Don't use installed_packages() for threejs URL
comment:4 Changed 2 years ago by
 Description modified (diff)
comment:5 in reply to: ↑ 2 Changed 2 years ago by
Replying to jdemeyer:
Replying to egourgoulhon:
Indeed the value of
installed_packages()['threejs']
is used to form the url in lines 749750 ofsrc/sage/repl/rich_output/display_manager.py
That's a really bad idea for multiple reasons.
Yes probably. I guess that was to ensure compatibility between the version of threejs actually used by Sage. By the way, in the directory upstream
, we have threejsr80.tar.gz
, so where does the .p0
come from?
comment:6 Changed 2 years ago by
 Cc ghantoniorojas added
comment:7 Changed 2 years ago by
That was indeed to ensure that the same version is used. If there is a better way  please implement it! And an obvious way to get rid of patched suffixes for this particular case is to just use the part of the version before the dot.
comment:8 Changed 2 years ago by
 Cc fbissey added
comment:9 Changed 2 years ago by
 Cc arojas added; ghantoniorojas removed
comment:10 Changed 2 years ago by
 Branch set to u/chapoton/25665
 Commit set to 867e1ffe5b15a3827d2828ad57ac23f392f7f4ee
 Status changed from new to needs_review
New commits:
867e1ff  trac 25665 fixing threejs URL

comment:11 Changed 2 years ago by
The use of is installed_packages
at runtime in sage is of course one of my pet hates. I don't have an alternative to offer at the present time. In fact I am just realising how this bit is broken in sageongentoo right now (I don't offer an offline option either). Where could I find a list of valid versions for the online cdn? Is there something like "latest"?
comment:12 Changed 2 years ago by
 Reviewers set to Eric Gourgoulhon
 Status changed from needs_review to positive_review
Thanks for the fix!
comment:13 Changed 2 years ago by
 Branch changed from u/chapoton/25665 to 867e1ffe5b15a3827d2828ad57ac23f392f7f4ee
 Resolution set to fixed
 Status changed from positive_review to closed
Replying to egourgoulhon:
That's a really bad idea for multiple reasons.