Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghost nodes for cylc graph. #2346

Merged

Conversation

oliver-sanders
Copy link
Member

Closes #2287

Graph view now displays tasks using ovals rather than boxes to match gcylc.

Before
before

After
after
ng)

@oliver-sanders oliver-sanders added this to the next release milestone Jul 6, 2017
@oliver-sanders oliver-sanders self-assigned this Jul 6, 2017
# Style regular (task) nodes.
node.attr['shape'] = 'oval'
if not any(seq.is_on_sequence(get_point(point)) for seq in
self.suiterc.taskdefs[name].sequences):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In essence of our recent validate optimisation, we know that sequence calculations are expensive. I wonder if it is worth caching the results here?

@oliver-sanders oliver-sanders force-pushed the 2287.ghost-nodes-for-graph-view branch 3 times, most recently from 4a109b4 to 2c21e4d Compare July 10, 2017 08:07
return False
else:
temp = sequence.is_on_sequence(get_point(point))
if cache:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work if cache is empty?

@matthewrmshin
Copy link
Contributor

(Branch in conflict.)

@oliver-sanders oliver-sanders force-pushed the 2287.ghost-nodes-for-graph-view branch from 03002df to 411a392 Compare July 11, 2017 14:33
@matthewrmshin
Copy link
Contributor

(./tests/graphing/09-close-fam.t failing on Travis CI.)

@oliver-sanders oliver-sanders force-pushed the 2287.ghost-nodes-for-graph-view branch 2 times, most recently from 98e69b0 to bf367f8 Compare July 14, 2017 15:19
@hjoliver
Copy link
Member

(conflicts)

@oliver-sanders oliver-sanders force-pushed the 2287.ghost-nodes-for-graph-view branch from bf367f8 to 3f18786 Compare July 25, 2017 11:32
@oliver-sanders
Copy link
Member Author

(resolutions)

@hjoliver
Copy link
Member

(heh heh 😁 )

node "fennel.1" "fennel\n1" unfilled box black
node "grape.1" "grape\n1" unfilled box black
node "artichoke.1" "artichoke\n1" unfilled ellipse black
node "arugula.1" "arugula\n1" unfilled ellipse black
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(box/ellipse ... and at first I was slightly concerned about the +/- 900 lines change size)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, its been fun changing all those lines!

@hjoliver
Copy link
Member

I thought you had broken right-click group/ungroup of families, but it turns out I'd checked out your other branch with identical branch name minus the final 'w' in '-view'! 1 test failing, but otherwise looks good to me (tested with nested families too).

@hjoliver
Copy link
Member

Can be merged if tests pass now. Over 'n out.

@hjoliver
Copy link
Member

Actually, you could mention this at the end of the CUG section "How The Graph Determines Task Instantiation".

@matthewrmshin
Copy link
Contributor

I'll take the liberty to merge this.

@matthewrmshin matthewrmshin merged commit 332ea35 into cylc:master Jul 25, 2017
matthewrmshin added a commit that referenced this pull request Aug 7, 2017
@oliver-sanders oliver-sanders deleted the 2287.ghost-nodes-for-graph-view branch December 14, 2017 12:06
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Jun 10, 2022
* Closes cylc#4638
* Fixes a regression where `cylc graph` was no longer
  rendering un-satisfiable dependencies differently
  (see cylc#2346 for original implementation).
* Restores the missing functionality to `cylc graph`
  and also adds warnings so un-satisfiable dependencies
  can be detected more easily.
* Refactors `cylc graph` a little, main change is that
  graphviz output is no longer achieved by re-processing
  the reference output using regexes. It's now generated
  from the graph directly.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Jun 10, 2022
* Closes cylc#4638
* Fixes a regression where `cylc graph` was no longer
  rendering un-satisfiable dependencies differently
  (see cylc#2346 for original implementation).
* Restores the missing functionality to `cylc graph`
  and also adds warnings so un-satisfiable dependencies
  can be detected more easily.
* Refactors `cylc graph` a little, main change is that
  graphviz output is no longer achieved by re-processing
  the reference output using regexes. It's now generated
  from the graph directly.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Jun 10, 2022
* Closes cylc#4638
* Fixes a regression where `cylc graph` was no longer
  rendering un-satisfiable dependencies differently
  (see cylc#2346 for original implementation).
* Restores the missing functionality to `cylc graph`
  and also adds warnings so un-satisfiable dependencies
  can be detected more easily.
* Refactors `cylc graph` a little, main change is that
  graphviz output is no longer achieved by re-processing
  the reference output using regexes. It's now generated
  from the graph directly.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Jun 14, 2022
* Closes cylc#4638
* Fixes a regression where `cylc graph` was no longer
  rendering un-satisfiable dependencies differently
  (see cylc#2346 for original implementation).
* Restores the missing functionality to `cylc graph`
  and also adds warnings so un-satisfiable dependencies
  can be detected more easily.
* Refactors `cylc graph` a little, main change is that
  graphviz output is no longer achieved by re-processing
  the reference output using regexes. It's now generated
  from the graph directly.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this pull request Jun 14, 2022
* Closes cylc#4638
* Fixes a regression where `cylc graph` was no longer
  rendering un-satisfiable dependencies differently
  (see cylc#2346 for original implementation).
* Restores the missing functionality to `cylc graph`
  and also adds warnings so un-satisfiable dependencies
  can be detected more easily.
* Refactors `cylc graph` a little, main change is that
  graphviz output is no longer achieved by re-processing
  the reference output using regexes. It's now generated
  from the graph directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants