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

Added instructions to install graphviz, if needed. #225

Merged
merged 2 commits into from
May 18, 2024

Conversation

AgnesBelt
Copy link
Contributor

Added instructions to install graphviz under docs/functionality.rst.

Description

I have just added few sentences in the documentation to flag the need for gaphviz to be installed, if the user wants to use the otoole viz functionality.

Issue Ticket Number

No issue raised.

Documentation

docs/functionality.rst

@trevorb1
Copy link
Member

Thanks for the PR, @AgnesBelt! We do mention how to install graphviz in the examples section -- however, neglect to show how to install it with conda, which is useful!. Do you think it would be useful to change the location of this warning to the core functionality, or leave in the examples? In any case, adding in the conda option is good!

https://otoole.readthedocs.io/en/latest/examples.html#otoole-visualise

image

@AgnesBelt
Copy link
Contributor Author

Oh, now I see it! I have totally missed the warning in the exercise section, sorry.
I would suggest to add the same warning to the core functionality section as well, as I believe it useful as a reminder in any case - as my experience shows, some people might only check in the core functionalities as they maybe are familiar with otoole already but they forgot how to use different commands/fucntionalities.
Also the brew install graphviz did not work for me on my Mac, so it would be definitely important to add in both places also the option with conda install graphviz as suggested earlier.

@trevorb1
Copy link
Member

Apologies for just getting back to this now, @AgnesBelt! Following your suggestions, I have:

  • Put a warning box in the core functionality that directs users to the visualization examples if they run into graphviz dependency issues. I linked the examples page, rather than putting the explicit install instructions in core functionality, just to limit repeat information for if it needs to get updated.
  • Added conda as an install method (thank you again for catching that!)
  • I have left brew as an install method, as that is still listed as the recommend method by Graphviz on Mac. If other users report this issue, we should probably report it to graphviz directly.

Additionally, just minor liniting is done to make this PR pass checks. Thanks again for your contribution! :)

@trevorb1 trevorb1 merged commit efe0e17 into OSeMOSYS:master May 18, 2024
7 checks passed
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.

2 participants