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

GTK/Gobject unnecessary for cylc graph --reference (7.8.x) #3343

Closed
sadielbartholomew opened this issue Sep 2, 2019 · 5 comments

Comments

@sadielbartholomew
Copy link
Member

commented Sep 2, 2019

Describe the bug

The cylc graph utility generally is interactive & requires relevant GUI application modules, but the --reference option in itself does not. However, the logic is written such that running with the --reference option will go through the standard list of module imports, including of gtk & gobject, terminating if these are not imported successfully:

cylc-flow/bin/cylc-graph

Lines 194 to 202 in 8ac7ae3

try:
import gtk
import gobject
from xdot import DotWindow
from cylc.cylc_xdot import (
MyDotWindow, MyDotWindow2, get_reference_from_plain_format)
except (ImportError, RuntimeError) as error:
# Allow command help generation without a graphical environment.
sys.exit('ERROR: no X environment? %s' % error)

So cylc graph --reference will fail in an environment without GTK, when it should according to inherent requirements work. Indeed, a user has expressed that it would be useful for them to use it without the need for GTK. Enabling it to do so requires only a minor restructure to the bin/cylc-graph code.

Release version(s) and/or repository branch(es) affected?

Not applicable for Cylc 8 & newer versions, where the graphing code has been replaced & GTK is not longer used in the codebase (🎉).

Steps to reproduce the bug

  1. Find or create an environment without either PyGTK (in Gnome-2) or PyGobject (Gnome-3) installed.

  2. You should observe by running the relevant command for some 'example-suite':

    $ cylc graph --reference <suite-name>
    ERROR: no X environment? No module named gtk

    (or gobject instead of gtk).

Expected behavior

cylc graph --reference runs as standard in an environment without GTK or Gobject.

Pull requests welcome!
This is an Open Source project - please consider contributing a bug fix
yourself (please read CONTRIBUTING.md before starting any work though).

@sadielbartholomew sadielbartholomew added this to the cylc-7.8.x milestone Sep 2, 2019

@sadielbartholomew sadielbartholomew changed the title cylc graph --reference should not need PyGTK/Gobject (7.8.x) PyGTK/Gobject unnecessary for cylc graph --reference (7.8.x) Sep 2, 2019

@sadielbartholomew sadielbartholomew changed the title PyGTK/Gobject unnecessary for cylc graph --reference (7.8.x) PyGTK/Gobject imported for cylc graph --reference (7.8.x) Sep 2, 2019

@sadielbartholomew sadielbartholomew changed the title PyGTK/Gobject imported for cylc graph --reference (7.8.x) GTK/Gobject unnecessary for cylc graph --reference (7.8.x) Sep 2, 2019

@hjoliver

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2019

I just took a quick look at the code. Unfortunately it's not as simple as avoiding the import test above if --reference is specified, because the reference graph is still written out via graph.draw() in the underlying xdot gtk application, which also imports gtk. Probably not too difficult to disintengtangle this if we really needed to, but I'm not sure it's worth the effort (for the reasons you gave on Discourse @sadielbartholomew - cylc 8 etc. Plus I don't see a user need for reference graphs - unlike a graphical representation they are harder to interpret than the suite.rc graph syntax itself).

So maybe we keep this issue to acknowledge the problem, but close it as "won't fix"?

@oliver-sanders

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

I re-implemented the --reference functionality for Cylc8 independent of pygtk. It's not too much code but really doesn't fit into the Cylc7 cylc-graph framework.

5b75c8c

Given the internal nature of the code and the timescale of Cylc8 can we punt this?

@matthewrmshin matthewrmshin removed this from the cylc-7.8.x milestone Sep 3, 2019

@hjoliver

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

I re-implemented the --reference functionality for Cylc8 independent of pygtk. It's not too much code but really doesn't fit into the Cylc7 cylc-graph framework.

I had forgotten about this, and it only took a few minutes to back-port it to Cylc 7 as a new command, so I'll reopen this and put up a PR ... given that there is (at least some) user demand.

@hjoliver hjoliver reopened this Sep 4, 2019

@hjoliver hjoliver removed the wontfix label Sep 4, 2019

@hjoliver hjoliver referenced this issue Sep 4, 2019
6 of 6 tasks complete
@sadielbartholomew

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

😕 This should have closed automatically with the merge of #3349, but it hasn't. I'm not sure why not, since that PR uses the standard 'close' syntax, but I'll close this manually.

@hjoliver

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

Thanks for closing this @sadielbartholomew - good spot 👍

Auto-close only works for PRs to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.