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

Remove obsolete visualization section. #4273

Merged
merged 8 commits into from
Jun 25, 2021

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jun 18, 2021

This is a small change with no associated Issue. Partially addresses #3379. May c-l-o-s-e #3696 (final check needed, of that umbrella issue).

  • remove obsolete [visualization] config section.
    • future Cylc 8 viz config will be different. The old config was very much tailored for graphviz.
  • simplify get_graph_raw() which was keeping track of family collapse state for the old UI. It now has a single argument to specify which families to collapse, and does not remember the collapsed state
  • modify cylc graph and cylc list, and functional tests that use them, accordingly (slightly enhanced too: they can take an interval for the stop point).

The PR would have been a lot smaller if I'd retained [visualization]initial/final cycle point which was being used by most of the functional tests that use cylc graph --reference ... but, better to remove the whole [visualization] section now. flow.cylc should define the whole graph, and cylc graph etc. should just specify what part of it they want to see.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.py and conda-environment.yml. [None]
  • Already covered by existing tests. (Removed some [vis] tests)
  • (master branch) I have opened a documentation PR at Remove obsoletion flow.cylc visualization section. cylc-doc#253

@hjoliver hjoliver added this to the cylc-8.0.0 milestone Jun 18, 2021
@hjoliver hjoliver self-assigned this Jun 18, 2021
@hjoliver hjoliver marked this pull request as draft June 18, 2021 04:32
@hjoliver
Copy link
Member Author

Draft while I fix tests that use visualization settings to define the graph extent.

@hjoliver hjoliver marked this pull request as ready for review June 21, 2021 11:26
@hjoliver
Copy link
Member Author

@oliver-sanders maybe one review will do as this is mostly functional test tweaks. Assign another reviewer if you like though.

@wxtim
Copy link
Member

wxtim commented Jun 22, 2021

Very likely that this closes #3696 - after this has gone in that will need a final check, and can then be closed... 🎉

@hjoliver
Copy link
Member Author

hjoliver commented Jun 22, 2021

Very likely that this closes #3696 - after this has gone in that will need a final check, and can then be closed... tada

Good spotting @wxtim - I'll update the description above.

[visualization] -> obsolete

@hjoliver hjoliver requested a review from wxtim June 22, 2021 10:17
Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

  • read it
  • tried it on random local workflows.

@oliver-sanders
Copy link
Member

Moved the milestone forward on #3696

@oliver-sanders oliver-sanders merged commit b7157fb into cylc:master Jun 25, 2021
@MetRonnie MetRonnie removed this from the cylc-8.0.0 milestone Jul 2, 2021
@MetRonnie MetRonnie added this to the cylc-8.0b2 milestone Jul 2, 2021
@hjoliver hjoliver deleted the rem-vis-section branch July 5, 2021 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants