Make cluster_seed a new-style spkg and rename it

[tscrim]

We make the cluster_seed into a new-style database spkg and rename it to database_mutation_class to give it a more accurate name.

tarball

CC: @egunawan @sagetrac-gmoose05 @vbraun @jdemeyer @stumpc5

Component: packages: optional

Author: Travis Scrimshaw

Branch/Commit: c49d60f

Reviewer: Frédéric Chapoton

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

[tscrim]

Branch: u/tscrim/cluster_seed_new_style_spkg-19177

[tscrim]

comment:1

Is there any place I need to change in the docs/scripts/etc. regarding the optional spkg name, or has that all become automated?

---

New commits:

c63b736

Make cluster_seed-1.0 into database_mutation_class-1.0.

[tscrim]

Commit: c63b736

[jdemeyer]

comment:2

Replying to @tscrim:

Is there any place I need to change in the docs/scripts/etc. regarding the optional spkg name, or has that all become automated?

No need to change anything.

Just a few small comments:

1. is there any doctest or Sage interface to check that the package works?
2. replace $SAGE_LOCAL/share by $SAGE_SHARE.
3. add a file build/pkgs/database_mutation_class/dependencies (which could be just saying # no dependencies)

[sagetrac-git]

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

b395554

Better practices based upon Jeroen's comments.

[sagetrac-git]

Changed commit from c63b736 to b395554

[tscrim]

comment:4

Replying to @jdemeyer:

Replying to @tscrim:

Is there any place I need to change in the docs/scripts/etc. regarding the optional spkg name, or has that all become automated?

No need to change anything.

Thanks for checking.

Just a few small comments:

1. is there any doctest or Sage interface to check that the package works?
2. replace $SAGE_LOCAL/share by $SAGE_SHARE.
3. add a file build/pkgs/database_mutation_class/dependencies (which could be just saying # no dependencies)

1. Yes, but it wasn't very explicit. I made it more so.
2. Done.
3. Done. Is this a new thing? Most of the spkg folders don't seem to have one unless they have an actual dependency.

[jdemeyer]

comment:5

Replying to @tscrim:

3. Done. Is this a new thing?

Well, it was introduced at the same that build/pkgs/$PKG/type was introduced. Which is pretty recent indeed.

Most of the spkg folders don't seem to have one unless they have an actual dependency.

It's not a disaster. The default for optional packages is that they depend on every standard package. It's a safe default, but not really efficient, especially given that sage -i will install dependencies soon (#19101).

[jdemeyer]

comment:6

Where is the package?

[tscrim]

Attachment: database_mutation_class-1.0.tar.gz

[tscrim]

comment:7

Whoops, forgot to upload that.

[jdemeyer]

comment:8

sage -t --long src/sage/combinat/cluster_algebra_quiver/mutation_type.py
**********************************************************************
File "src/sage/combinat/cluster_algebra_quiver/mutation_type.py", line 1226, in sage.combinat.cluster_algebra_quiver.mutation_type.load_data
Failed example:
    load_data(1)
Expected:
    {}
Got:
    {('A', 1): [('@?', ())]}
**********************************************************************
File "src/sage/combinat/cluster_algebra_quiver/mutation_type.py", line 1228, in sage.combinat.cluster_algebra_quiver.mutation_type.load_data
Failed example:
    load_data(2) # optional database_mutation_class
Expected:
    {('G', 2): [('AO', (((0, 1), (1, -3)),)), ('AO', (((0, 1), (3, -1)),))]}
Got:
    {('A', 2): [('AO', ())],
     ('A', (1, 1), 1): [('AO', (((0, 1), (2, -2)),))],
     ('B', 2): [('AO', (((0, 1), (1, -2)),)), ('AO', (((0, 1), (2, -1)),))],
     ('BC', 1, 1): [('AO', (((0, 1), (1, -4)),)), ('AO', (((0, 1), (4, -1)),))],
     ('G', 2): [('AO', (((0, 1), (1, -3)),)), ('AO', (((0, 1), (3, -1)),))]}
**********************************************************************

[jdemeyer]

comment:10

Also, I get the same doctest output with or without this package, what is going on?

[sagetrac-git]

Changed commit from b395554 to 61c7f82

[sagetrac-git]

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

61c7f82

Fixing doctest failures.

[tscrim]

comment:12

What is happening is that there is a test in one of the other files which saves some data to your .sage folder. This should fix things and isolates the tests specific to the database.

Christian, I'm cc-ing you in case you have any opinions on the name of the database (and to let you know that it is changing).

[sagetrac-git]

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

c49d60f

Fix optional tag.

[sagetrac-git]

Changed commit from 61c7f82 to c49d60f

[jdemeyer]

comment:14

Isn't this actually a bug?

# random - depends on the data the user has stored

If load_data() returns essentially random data, how can it be useful?

It wasn't the test which was broken (hence, no need for test_database), the function itself was broken.

[tscrim]

comment:15

I don't consider it as one because it depends on if the user has already stored information for the rank 1 types or not. If the user hasn't computed and saved anything (or installed the database), then there is nothing to load. This is why you were getting the same output before and after, whereas I was getting less data in the output. (It also can't raise an error because it looks in 2 places for file data.)

[jdemeyer]

comment:16

Replying to @tscrim:

I don't consider it as one because it depends on if the user has already stored information for the rank 1 types or not.

I think it's a bug that it does depend on that.

Anyway, I don't care much about this package. I will not set this ticket to needs_work for this, but I also won't set it to positive_review either.

[fchapoton]

comment:18

For the patchbot, it is probably better if the tarball url appears in full.

[fchapoton]

Reviewer: Frédéric Chapoton

[fchapoton]

comment:21

tested, and good to go

[vbraun]

Changed branch from u/tscrim/cluster_seed_new_style_spkg-19177 to c49d60f