Skip to content

[ENH] Sphinx extension to plot workflows #1896

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

Merged
merged 11 commits into from
Mar 23, 2017

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Mar 21, 2017

Adds a sphinx extension to the nipype installation. After adding nipype.sphinxext.plot_workflow to the extensions list in the doc/conf.py, sphinx will parse blocks like:

.. workflow ::
    :graph2use: flat
    :simple_form: no

    from nipype.workflows.dmri.camino.connectivity_mapping import create_connectivity_pipeline
    wf = create_connectivity_pipeline()

The graph is embedded at the point where the block is placed in the docstring.

@oesteban oesteban changed the title [ENH] Sphinx extension to plot workflows [WIP/ENH] Sphinx extension to plot workflows Mar 21, 2017
@oesteban oesteban changed the title [WIP/ENH] Sphinx extension to plot workflows [ENH] Sphinx extension to plot workflows Mar 22, 2017
@oesteban
Copy link
Contributor Author

@oesteban oesteban requested a review from satra March 22, 2017 02:23
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Generally looks fine. In spinxext/plot_workflow.py, I got as far as run_code before I found my attention wandering. Can't guarantee there are no problems I missed before that, but nothing glaring.

"""
Equivalent to bash's mkdir -p
"""
if not os.path.exists(folder):
Copy link
Member

Choose a reason for hiding this comment

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

You might want to use os.path.isdir, which will throw an EEXIST if a non-directory exists.

@codecov-io
Copy link

codecov-io commented Mar 22, 2017

Codecov Report

Merging #1896 into master will decrease coverage by 2.34%.
The diff coverage is 16.51%.

@@            Coverage Diff             @@
##           master    #1896      +/-   ##
==========================================
- Coverage   72.92%   70.57%   -2.35%     
==========================================
  Files        1061     1059       -2     
  Lines       53749    53688      -61     
  Branches     7728     7791      +63     
==========================================
- Hits        39194    37888    -1306     
- Misses      13341    14416    +1075     
- Partials     1214     1384     +170
Flag Coverage Δ
#smoketests 70.57% <16.51%> (-2.35%) ⬇️
#unittests 70.02% <16.51%> (-0.34%) ⬇️
Impacted Files Coverage Δ
nipype/info.py 83.6% <ø> (ø) ⬆️
nipype/pipeline/engine/workflows.py 74.78% <100%> (-1.63%) ⬇️
nipype/sphinxext/__init__.py 100% <100%> (ø)
nipype/sphinxext/plot_workflow.py 13.22% <13.22%> (ø)
nipype/utils/filemanip.py 80.75% <22.22%> (-4%) ⬇️
nipype/pipeline/engine/utils.py 76.7% <77.77%> (-3.74%) ⬇️
nipype/interfaces/spm/model.py 41.76% <0%> (-29.98%) ⬇️
nipype/algorithms/rapidart.py 35.39% <0%> (-28.91%) ⬇️
nipype/interfaces/fsl/model.py 53.89% <0%> (-26.47%) ⬇️
nipype/workflows/fmri/fsl/preprocess.py 72.77% <0%> (-15.19%) ⬇️
... and 26 more

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 cf2b1f8...1f6335b. Read the comment docs.

Equivalent to bash's mkdir -p
"""
try:
os.makedirs(folder)
Copy link
Member

Choose a reason for hiding this comment

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

on py3 we should just use os.makedirs(exist_ok=True), (also see bug that was fixed here: https://bugs.python.org/issue21082)

for py2, i'm still worried about race conditions, given that this is something people can start using. how about, just make this a hidden function for the moment and use it in the extension?

@satra satra merged commit 0547f67 into nipy:master Mar 23, 2017
@oesteban oesteban deleted the enh/SphinxExtension-workflows branch May 1, 2017 01:46
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