601: gscan groups #2024

arjclark · 2016-10-04T14:25:35Z

Adds "group" config item to allow collapsing groups of suites in gscan.

A simple suite like this:

title = "Test Suite"
description="Testing grouping options"
group="test group"

[scheduling]
[[dependencies]]
graph = foo

will appear under a "Group" heading of "test group", if displayed in gscan.

@oliver-sanders - can you take an initial cursory look as someone else who's had to work with this recently? (also includes the indexing "fix" discussed)

Not ready for full review/merge yet.

hjoliver · 2016-10-10T23:45:15Z

I wonder if it would be better to define grouping in the user's gscan.rc, for the following reasons:

these groups only affect display in gscan (at least I can't think of other uses at this stage...)
you can see all group members at a glance (else, have to grep through all suite definitions?)
easy to use this new functionality without editing existing (and potentially running) suites

hjoliver · 2016-10-10T23:49:31Z

This looks good, on a quick test. I wonder if we could have a combined status summary for all group members though? (may be too difficult, just wondering...)

arjclark · 2016-10-11T07:58:43Z

I wonder if we could have a combined status summary for all group members though

It's certainly something I'm planning on trying to do - there's a few bits that'll be a pain to do given how we extract status information from suites but I don't see it as not being possible.

arjclark · 2016-10-11T08:02:18Z

I wonder if it would be better to define grouping in the user's gscan.rc, for the following reasons:

these groups only affect display in gscan (at least I can't think of other uses at this stage...)

you can see all group members at a glance (else, have to grep through all suite definitions?)

easy to use this new functionality without editing existing (and potentially running) suites

I can see both sides of this tbh. @cylc/core - any preferences either way and if we went down @hjoliver's route how you'd ideally like to see this configured?

arjclark · 2016-10-11T08:50:52Z

So, we've had a bit of a chat on our end (primarily me and @dpmatthews) and can see both arguments for where the configuration option should be. On the one hand, putting it in the suite.rc seems the natural place to put it and exposes the setting to the user rather than burying it away in a gcylc.rc file where it risks being forgotten about/not used and becomes an extra file to modify etc. On the other having it in the gcylc file would mean you could easily change the grouping for running suites after, say, spinning up a set of operational suites and then having to subsequently reload them to apply a change in grouping.

In the long term there's no reason why we couldn't support both with e.g. gcylc.rc providing default group values which then suite.rc entries could override. For now though, we'd prefer to go with the solution here and then, based on uptake/usage, we could extend to the gcylc.rc as well in a future pull request. Does this sound ok to you @hjoliver?

hjoliver · 2016-10-11T19:45:27Z

Yep, that's fine. I like the idea of defaulting to settings in gscan.rc (if any), but that can be another PR.

arjclark · 2016-10-31T12:26:58Z

Coming back round to this now - been distracted elsewhere. Working on trying to get group summaries up and running.

arjclark · 2016-11-08T14:14:57Z

In theory all working now.

@oliver-sanders @hjoliver please review 1 and 2 respectively.

oliver-sanders · 2016-11-08T17:10:31Z

I'm getting this for the example in the original comment.

Am I doing something wrong?

oliver-sanders · 2016-11-09T09:16:15Z

t/pep8 is failing.

arjclark · 2016-11-09T11:07:20Z

@oliver-sanders - sorry rebased and broke things. Should be fine now.

oliver-sanders · 2016-11-09T12:03:56Z

I've got some strange issues with status icons appearing as black boxes also some (possibly un-related) traceback.

benfitzpatrick · 2016-11-09T14:34:45Z

Those black boxes are ‘unknown’ status, FYI

From: Oliver Sanders [mailto:notifications@github.com]
Sent: 09 November 2016 12:04
To: cylc/cylc
Cc: Fitzpatrick, Ben; Team mention
Subject: Re: [cylc/cylc] 601: gscan groups (#2024)

I've got some strange issues with status icons appearing as black boxes also some (possibly un-related) traceback.

[screenshot-cylc gscan-1]https://cloud.githubusercontent.com/assets/16705946/20137754/4e3cfd6a-a674-11e6-902b-a382da8fc628.png

[screenshot-untitled window-1]https://cloud.githubusercontent.com/assets/16705946/20137765/620878b0-a674-11e6-8189-8c7772bf26c4.png

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/2024#issuecomment-259399423, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABFenTnEbUXZHrWpsscs2Q7uB5dk5VRXks5q8basgaJpZM4KNvZ_.

arjclark · 2016-11-10T12:24:09Z

@oliver-sanders - black box issue resolved, along with a few others.

arjclark · 2016-11-10T12:24:38Z

Cannot reproduce the traceback problem - I believe this isn't a result of this change though.

oliver-sanders

Works great, code looks fine. Couple of very minor points.

oliver-sanders · 2016-11-10T14:15:20Z

lib/cylc/gui/gscan.py

@@ -1123,10 +1181,35 @@ def update(self):
            )
            title = suite_info.get("title")

+            group = suite_info.get("group")
+            if group is None:
+                group = ""


group="Un-grouped" might be more helpful.

oliver-sanders · 2016-11-10T14:22:29Z

lib/cylc/gui/gscan.py

@@ -1144,27 +1227,31 @@ def update(self):
                        if state != TASK_STATUS_RUNAHEAD:
                            # 'runahead' states are usually hidden.
                            states_text += '%s %d ' % (state, number)
+                            # Extractable counts here?


As it turned out, no

oliver-sanders · 2016-11-10T14:36:09Z

Moved traceback issue to #2061.

arjclark · 2016-11-10T15:54:55Z

Feedback from @oliver-sanders addressed. Over to you @hjoliver (or @matthewrmshin if you want to take it)

matthewrmshin

Minor comments. OK otherwise.

matthewrmshin · 2016-11-16T12:35:53Z

lib/cylc/gui/gscan.py

@@ -1123,10 +1181,35 @@ def update(self):
            )
            title = suite_info.get("title")

+            group = suite_info.get("group")
+            if group is None:
+                group = "ungrouped"


I wonder if a string like (default) is better here?

I think "ungrouped" is clearer.

In which case, can we do (ungrouped) in bracket to make it clearer that we have not got a group called ungrouped?

matthewrmshin · 2016-11-16T12:38:10Z

lib/cylc/cfgspec/suite.py

@@ -203,6 +203,7 @@ def _coerce_parameter_list(value, keys, _):
 SPEC = {
    'title': vdr(vtype='string', default=""),
    'description': vdr(vtype='string', default=""),
+    'group': vdr(vtype='string', default=""),


Worth adding a simple validate test for this new setting?

What did you have in mind?

Conflicts: lib/cylc/gui/gscan.py

arjclark · 2016-11-17T10:34:56Z

@matthewrmshin - default group is now "(ungrouped)". Validation test added.

hjoliver · 2016-11-23T09:32:04Z

(Nice work y'all)

arjclark added this to the soon milestone Oct 4, 2016

arjclark changed the title ~~601b.gscan groups~~ 601: gscan groups Oct 4, 2016

arjclark self-assigned this Oct 19, 2016

basic group functionality implemented.

072b8da

arjclark force-pushed the 601b.gscan_groups branch from a0d65b0 to 072b8da Compare November 8, 2016 14:14

arjclark assigned hjoliver and oliver-sanders Nov 8, 2016

arjclark modified the milestones: next release, soon Nov 8, 2016

arjclark removed their assignment Nov 8, 2016

arjclark added 2 commits November 9, 2016 11:04

Fix missing key error.

62a01cf

Fix pep8

a46133f

arjclark added 2 commits November 10, 2016 12:21

Fix black boxes and missing info.

bd96165

pep8 fix.

1c17131

oliver-sanders reviewed Nov 10, 2016

View reviewed changes

Review feedback.

ae8701e

oliver-sanders removed their assignment Nov 14, 2016

matthewrmshin assigned matthewrmshin and unassigned hjoliver Nov 16, 2016

matthewrmshin reviewed Nov 16, 2016

View reviewed changes

arjclark added 2 commits November 17, 2016 09:41

Merge remote branch 'upstream/master' into 601b.gscan_groups

1a0f302

Conflicts: lib/cylc/gui/gscan.py

Add test and set default as "(ungrouped)"

7030712

arjclark force-pushed the 601b.gscan_groups branch from 2a85934 to 7030712 Compare November 17, 2016 10:33

matthewrmshin merged commit 26fa00a into cylc:master Nov 17, 2016

arjclark deleted the 601b.gscan_groups branch November 22, 2016 08:41

oliver-sanders mentioned this pull request Jan 11, 2017

collapsible named groups of suites in gscan gui? #601

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

601: gscan groups #2024

601: gscan groups #2024

arjclark commented Oct 4, 2016 •

edited

Loading

hjoliver commented Oct 10, 2016 •

edited

Loading

hjoliver commented Oct 10, 2016

arjclark commented Oct 11, 2016

arjclark commented Oct 11, 2016

arjclark commented Oct 11, 2016

hjoliver commented Oct 11, 2016

arjclark commented Oct 31, 2016

arjclark commented Nov 8, 2016

oliver-sanders commented Nov 8, 2016

oliver-sanders commented Nov 9, 2016

arjclark commented Nov 9, 2016 •

edited

Loading

oliver-sanders commented Nov 9, 2016

benfitzpatrick commented Nov 9, 2016

arjclark commented Nov 10, 2016

arjclark commented Nov 10, 2016

oliver-sanders left a comment

oliver-sanders Nov 10, 2016

oliver-sanders Nov 10, 2016

arjclark Nov 10, 2016

oliver-sanders commented Nov 10, 2016

arjclark commented Nov 10, 2016

matthewrmshin left a comment

matthewrmshin Nov 16, 2016

oliver-sanders Nov 16, 2016

matthewrmshin Nov 16, 2016

matthewrmshin Nov 16, 2016

arjclark Nov 16, 2016

arjclark commented Nov 17, 2016

hjoliver commented Nov 23, 2016

601: gscan groups #2024

601: gscan groups #2024

Conversation

arjclark commented Oct 4, 2016 • edited Loading

hjoliver commented Oct 10, 2016 • edited Loading

hjoliver commented Oct 10, 2016

arjclark commented Oct 11, 2016

arjclark commented Oct 11, 2016

arjclark commented Oct 11, 2016

hjoliver commented Oct 11, 2016

arjclark commented Oct 31, 2016

arjclark commented Nov 8, 2016

oliver-sanders commented Nov 8, 2016

oliver-sanders commented Nov 9, 2016

arjclark commented Nov 9, 2016 • edited Loading

oliver-sanders commented Nov 9, 2016

benfitzpatrick commented Nov 9, 2016

arjclark commented Nov 10, 2016

arjclark commented Nov 10, 2016

oliver-sanders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliver-sanders commented Nov 10, 2016

arjclark commented Nov 10, 2016

matthewrmshin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arjclark commented Nov 17, 2016

hjoliver commented Nov 23, 2016

arjclark commented Oct 4, 2016 •

edited

Loading

hjoliver commented Oct 10, 2016 •

edited

Loading

arjclark commented Nov 9, 2016 •

edited

Loading