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

Update docstring to illustrate plotting dependency graph in Jupyter notebook #791

Merged
merged 7 commits into from
May 16, 2019

Conversation

maxalbert
Copy link
Contributor

Closes #786

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

@maxalbert maxalbert added the FlowMachine Issues related to FlowMachine label May 15, 2019
@greenape
Copy link
Member

Doesn't render quite right in the docs.

Screenshot 2019-05-15 at 16 38 44

@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #791 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #791   +/-   ##
=======================================
  Coverage   93.14%   93.14%           
=======================================
  Files         130      130           
  Lines        6523     6523           
  Branches      691      691           
=======================================
  Hits         6076     6076           
  Misses        327      327           
  Partials      120      120
Impacted Files Coverage Δ
flowmachine/flowmachine/utils.py 45.56% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b807c0...b417eca. Read the comment docs.

@maxalbert maxalbert added ready-to-merge Label indicating a PR is OK to automerge and removed ready-to-merge Label indicating a PR is OK to automerge labels May 15, 2019
@maxalbert
Copy link
Contributor Author

@greenape Do you know how to fix this? Is it the same \b trick that you used in #785? Or should I remove one level of indentation for the examples? Or something else?

@greenape
Copy link
Member

Looks like the indent is what's causing the doctest formatted examples to not get rendered right.

Probably also want to make the Graphviz link a markdown link as well, and maybe wrap the bash command in backticks?

@maxalbert
Copy link
Contributor Author

maxalbert commented May 15, 2019

I updated the docstring and the Python code is now correctly formatted, but the bash command still appears inline and is wrapped up with the link to graphviz at the end. Any idea how to fix this?

Screen Shot 2019-05-15 at 17 37 10

Copy link
Member

@greenape greenape left a comment

Choose a reason for hiding this comment

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

👍

@maxalbert maxalbert added the ready-to-merge Label indicating a PR is OK to automerge label May 16, 2019
@mergify mergify bot merged commit 9ceb1f9 into master May 16, 2019
@mergify mergify bot deleted the update-docstring branch May 16, 2019 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FlowMachine Issues related to FlowMachine ready-to-merge Label indicating a PR is OK to automerge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Easier visualisation of dependency graph?
2 participants