Opened 4 years ago
Closed 2 years ago
#28022 closed task (fixed)
Downgrade rubiks to optional
Reported by:  isuruf  Owned by:  

Priority:  critical  Milestone:  sage9.2 
Component:  packages: standard  Keywords:  
Cc:  ghtimokau, jhpalmieri  Merged in:  
Authors:  Frédéric Chapoton, John Palmieri  Reviewers:  Dima Pasechnik, Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  a8ea381 (Commits, GitHub, GitLab)  Commit:  a8ea38136af94bf60af6197845597ba17ab2fe95 
Dependencies:  Stopgaps: 
Description
Change History (30)
comment:1 Changed 4 years ago by
Branch:  → public/ticket/28022 

Commit:  → 0da247981cdc1891a3334abf9501886a51c2e605 
Status:  new → needs_review 
comment:2 Changed 4 years ago by
Cc:  ghtimokau added 

comment:3 Changed 4 years ago by
this also needs an update of http://wrongway.org/?rubiksource
in rubiks.py, as it's not working any more (domain is up for sale).
comment:4 Changed 4 years ago by
Commit:  0da247981cdc1891a3334abf9501886a51c2e605 → 9a9087a11b046f3a438922abd7956742641e96fb 

Branch pushed to git repo; I updated commit sha1. New commits:
9a9087a  trac 28022 change dead url to web archive version

comment:7 Changed 4 years ago by
If we're not changing the package, there is no need to edit build/pkgs/rubiks/packageversion.txt
comment:8 Changed 4 years ago by
Commit:  9a9087a11b046f3a438922abd7956742641e96fb → fbc54a19c90b2e97837052c0f9813b96a736b8eb 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
fbc54a1  trac 28022 downgrade rubiks to optional

comment:10 Changed 4 years ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_review → positive_review 
OK, don't forget the author's name.
comment:12 Changed 4 years ago by
Status:  positive_review → needs_work 

We forgot src/sage/groups/perm_gps/cubegroup.py
, where rubiks
is used unconditionally.
comment:13 Changed 4 years ago by
Commit:  fbc54a19c90b2e97837052c0f9813b96a736b8eb → 1ecfad60e0efd3fd9bee355415fa7fa9a04fbe0c 

comment:15 Changed 4 years ago by
Status:  needs_review → needs_work 

this seems to be a wrong (always succeeding) test for the presense of rubiks package:
try: import sage.interfaces.rubik # here to avoid circular referencing except ImportError: algorithm = 'gap'
indeed, the interface is always there, whether or not rubiks is installed.
comment:16 Changed 4 years ago by
ok.. Then I have no idea what to do. If you know how to fix that, please do..
comment:17 Changed 4 years ago by
How about simply calling is_package_installed('rubiks')
?
sage: is_package_installed('rubiks') False
comment:18 Changed 4 years ago by
Note that is_package_installed
is supposed to be deprecated: https://trac.sagemath.org/ticket/20382, mostly (I think) because it is a pain for packagers.
comment:19 Changed 4 years ago by
Features
entails creating sage/features/rubiks.py
with some runtime tests... looks a bit insane, but OK...
comment:20 Changed 4 years ago by
According to the file manifest, the rubiks
package only installs six binaries, and it shouldn't be too bad to check that they exist and work, for the Features
test. You could model it on sage/features/graphviz.py
, for example.
comment:21 Changed 4 years ago by
+1 to adding it to sage.features
. This is what I suggested in the first place :)
comment:23 Changed 3 years ago by
Milestone:  sage8.9 → sage9.1 

Ticket retargeted after milestone closed
comment:24 Changed 3 years ago by
Milestone:  sage9.1 → sage9.2 

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.
comment:25 Changed 2 years ago by
Cc:  jhpalmieri added 

comment:26 Changed 2 years ago by
Priority:  major → critical 

comment:27 Changed 2 years ago by
Commit:  1ecfad60e0efd3fd9bee355415fa7fa9a04fbe0c → a8ea38136af94bf60af6197845597ba17ab2fe95 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a8ea381  trac 28022: rebased Frédéric Chapoton's branch, added Features support

comment:28 Changed 2 years ago by
Authors:  Frédéric Chapoton → Frédéric Chapoton, John Palmieri 

Status:  needs_work → needs_review 
I rebased Frédéric's work, and I added a file features/rubiks.py
. Please test with and without installing the rubiks
package.
comment:29 Changed 2 years ago by
Reviewers:  Dima Pasechnik → Dima Pasechnik, Matthias Koeppe 

Status:  needs_review → positive_review 
This works well.
comment:30 Changed 2 years ago by
Branch:  public/ticket/28022 → a8ea38136af94bf60af6197845597ba17ab2fe95 

Resolution:  → fixed 
Status:  positive_review → closed 
here is first tentative
New commits:
trac 28022 downgrade rubiks to optional