Opened 3 years ago

Closed 2 years ago

#28022 closed task (fixed)

Downgrade rubiks to optional

Reported by: Isuru Fernando Owned by:
Priority: critical Milestone: sage-9.2
Component: packages: standard Keywords:
Cc: Timo Kaufmann, John Palmieri 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:

Status badges

Change History (30)

comment:1 Changed 3 years ago by Frédéric Chapoton

Branch: public/ticket/28022
Commit: 0da247981cdc1891a3334abf9501886a51c2e605
Status: newneeds_review

here is first tentative


New commits:

0da2479trac 28022 downgrade rubiks to optional

comment:2 Changed 3 years ago by Timo Kaufmann

Cc: Timo Kaufmann added

comment:3 Changed 3 years ago by Dima Pasechnik

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).

e.g. we can link to https://web.archive.org/web/20121212175710/http://www.wrongway.org/?rubiksource

Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:4 Changed 3 years ago by git

Commit: 0da247981cdc1891a3334abf9501886a51c2e6059a9087a11b046f3a438922abd7956742641e96fb

Branch pushed to git repo; I updated commit sha1. New commits:

9a9087atrac 28022 change dead url to web archive version

comment:5 Changed 3 years ago by Frédéric Chapoton

done

comment:6 Changed 3 years ago by Erik Bray

+1 Would it make sense to use sage.features for this?

comment:7 Changed 3 years ago by Jeroen Demeyer

If we're not changing the package, there is no need to edit build/pkgs/rubiks/package-version.txt

comment:8 Changed 3 years ago by git

Commit: 9a9087a11b046f3a438922abd7956742641e96fbfbc54a19c90b2e97837052c0f9813b96a736b8eb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

fbc54a1trac 28022 downgrade rubiks to optional

comment:9 Changed 3 years ago by Frédéric Chapoton

version bump was undone, and then the branch squashed

comment:10 Changed 3 years ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

OK, don't forget the author's name.

comment:11 Changed 3 years ago by Frédéric Chapoton

Authors: Frédéric Chapoton

thx

comment:12 Changed 3 years ago by Dima Pasechnik

Status: positive_reviewneeds_work

We forgot src/sage/groups/perm_gps/cubegroup.py, where rubiks is used unconditionally.

comment:13 Changed 3 years ago by git

Commit: fbc54a19c90b2e97837052c0f9813b96a736b8eb1ecfad60e0efd3fd9bee355415fa7fa9a04fbe0c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5f27079trac 28022 downgrade rubiks to optional
1ecfad6trac 28022 adding more optional rubiks tags

comment:14 Changed 3 years ago by Frédéric Chapoton

Status: needs_workneeds_review

indeed. Here is a fix.

comment:15 Changed 3 years ago by Dima Pasechnik

Status: needs_reviewneeds_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 3 years ago by Frédéric Chapoton

ok.. Then I have no idea what to do. If you know how to fix that, please do..

comment:17 Changed 3 years ago by Dima Pasechnik

How about simply calling is_package_installed('rubiks') ?

sage: is_package_installed('rubiks')
False

comment:18 Changed 3 years ago by Timo Kaufmann

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 3 years ago by Dima Pasechnik

Features entails creating sage/features/rubiks.py with some runtime tests... looks a bit insane, but OK...

comment:20 Changed 3 years ago by John Palmieri

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 3 years ago by Erik Bray

+1 to adding it to sage.features. This is what I suggested in the first place :)

comment:22 Changed 3 years ago by Frédéric Chapoton

I will not be the one that does that.

comment:23 Changed 3 years ago by Erik Bray

Milestone: sage-8.9sage-9.1

Ticket retargeted after milestone closed

comment:24 Changed 3 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.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 Matthias Köppe

Cc: John Palmieri added

comment:26 Changed 2 years ago by Matthias Köppe

Priority: majorcritical

comment:27 Changed 2 years ago by git

Commit: 1ecfad60e0efd3fd9bee355415fa7fa9a04fbe0ca8ea38136af94bf60af6197845597ba17ab2fe95

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a8ea381trac 28022: rebased Frédéric Chapoton's branch, added Features support

comment:28 Changed 2 years ago by John Palmieri

Authors: Frédéric ChapotonFrédéric Chapoton, John Palmieri
Status: needs_workneeds_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 Matthias Köppe

Reviewers: Dima PasechnikDima Pasechnik, Matthias Koeppe
Status: needs_reviewpositive_review

This works well.

comment:30 Changed 2 years ago by Volker Braun

Branch: public/ticket/28022a8ea38136af94bf60af6197845597ba17ab2fe95
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.