-
Notifications
You must be signed in to change notification settings - Fork 60
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
[fix] Added graphviz dependency (Resolves #637) #660
[fix] Added graphviz dependency (Resolves #637) #660
Conversation
Added graphviz to requirements.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. A few minor comments.
doc/requirements.txt
Outdated
@@ -20,6 +20,7 @@ docutils==0.20.1 | |||
# myst-parser | |||
# sphinx | |||
# sphinx-rtd-theme | |||
graphviz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually needs to go here and then we use pip-compile
to generate the actual requirements.txt
from there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmadlener Thanks for explaining, I've updated requirements.in
. I'm sorry for adding the dependency to an auto-generated file 😅
Sorry, this might have been a bit confusing if you are not completely familiar with podios structure. There are actually two
Hence, what you want to change for fixing the issue is the first one and not the |
ec797b3
to
e992780
Compare
doc/requirements.in
Outdated
@@ -2,4 +2,4 @@ breathe | |||
myst-parser | |||
sphinx | |||
sphinx_rtd_theme | |||
sphinx_copybutton | |||
sphinx_copybutton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sphinx_copybutton | |
sphinx_copybutton |
Simply to not touch this file in this PR.
edit: github doesn't really deal with this properly. the fix is to add a newline after sphinx_copybutton
(same for the suggestion below).
requirements.txt
Outdated
@@ -2,3 +2,4 @@ | |||
pyyaml | |||
jinja2 | |||
tabulate | |||
graphviz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
graphviz | |
graphviz |
Something in your editor settings seems to strip off the last newline in each file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. I'll check for the new lines from next time. Thanks!
This reverts commit b50358a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your work (and patience). Waiting for the CI to run to completion before merging.
Added graphviz to requirements.txt
Resolves #637
BEGINRELEASENOTES
graphviz
forpodio-vis
functionality in the documentation build process.ENDRELEASENOTES