Skip to content

DOC: Move users docs from nipype to nipype_tutorial #2726

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 6 commits into from
Oct 31, 2018

Conversation

miykael
Copy link
Collaborator

@miykael miykael commented Oct 10, 2018

Summary

PR miykael/nipype_tutorial#77 and miykael/nipype_tutorial#128 moved the content under User Guide already to the nipype_tutorial, but we haven't removed this content from nipype, yet. This PR takes care of that.

Deviation between the content

There are three sections that have been copied to the tutorial but either leads to errors or are still WIP:

Saving Workflows and Nodes to a file (experimental)

Running wf.export('special_workflow.py') in the notebook will lead to a TypeError: a bytes-like object is required, not 'str'. As the content of this section is still "experimental", I think it's ok if we leave it as WIP.

Running Nipype Interfaces from the command line (nipype_cmd)

The content between nipype and nipype_tutorial is identical. But currently, we're not running any tests, as the command nipype_cmd doesn't work inside the docker image. I haven't used nipype_cmd yet myself, but it seems nipype needs to be installed via apt install python-nipype for this to work?

Resource Scheduling and Profiling with Nipype

The content between nipype and nipype_tutorial is identical, but running the profiler within the docker image leads to the error AttributeError: 'list' object has no attribute 'startTime'. This is caused by the fact that the profiling information is not handed over as an object but as an object inside a list.

To consider

Removing the "User Docs" from nipype leads to new issues that need to be considered:

  1. install.rst wasn't removed from doc/users as it is also needed in nipype's Quickstart section.
  2. doc/users contains a folder examples that contains the workflow examples under Interfaces, Workflows and Examples. Once those workflows are included under niflows, this folder can be removed.
  3. This PR inverts the order between the "Developer Guides" and the "Interfaces, Workflows and Examples" section, to this:
    new_documentation_layout
  4. This PR puts the API and Developer Guide into two columns (see below). But the current setup has titles, as well as links to the corresponding sections. Should we take care of this?
    developer_title

@@ -16,7 +16,8 @@ image from Docker hub::
docker pull nipype/nipype

You may also build custom docker containers with specific versions of software
using Neurodocker_ (see the :doc:`neurodocker`).
using Neurodocker_ (see the `Neurodocker tutorial
<https://miykael.github.io/nipype_tutorial/notebooks/introduction_neurodocker.html>`_).
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to nipype.github.io/tutorial? Not sure if we already had discussion there...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're talking about the whole nipype_tutorial? Fine with me, but we haven't discussed it yet, or decided what to do.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah.

@effigies effigies added this to the 1.1.4 milestone Oct 11, 2018
@effigies
Copy link
Member

This seems fine to me. Are there any outstanding issues? Can you merge master to get the latest version of testing in?

One quick note: nipype_cmd is now nipypecli. Not sure if that makes a difference for your notebook...

@effigies
Copy link
Member

Actually, would you mind pushing a build of the docs to your gh-pages, so we can check them out on https://miykael.github.io/nipype?

@codecov-io
Copy link

Codecov Report

Merging #2726 into master will decrease coverage by 3.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2726     +/-   ##
=========================================
- Coverage   67.44%   64.24%   -3.2%     
=========================================
  Files         340      338      -2     
  Lines       43164    43102     -62     
  Branches     5351     5348      -3     
=========================================
- Hits        29110    27692   -1418     
- Misses      13355    14333    +978     
- Partials      699     1077    +378
Flag Coverage Δ
#smoketests ?
#unittests 64.24% <ø> (-0.57%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/utils/spm_docs.py 25.92% <0%> (-44.45%) ⬇️
nipype/interfaces/freesurfer/base.py 49.59% <0%> (-30.9%) ⬇️
nipype/utils/logger.py 59.7% <0%> (-29.86%) ⬇️
nipype/algorithms/rapidart.py 35.39% <0%> (-29.21%) ⬇️
nipype/interfaces/spm/base.py 58.41% <0%> (-29.05%) ⬇️
nipype/utils/provenance.py 55.73% <0%> (-28.99%) ⬇️
nipype/interfaces/fsl/model.py 55.26% <0%> (-25.35%) ⬇️
nipype/testing/fixtures.py 77.33% <0%> (-21.34%) ⬇️
nipype/interfaces/matlab.py 64.64% <0%> (-21.22%) ⬇️
... and 46 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 39675df...93c475b. Read the comment docs.

@miykael
Copy link
Collaborator Author

miykael commented Oct 25, 2018

I merged with master and renamed nipype_cmd to nipypecli. The newest version of the docs can be found on my gh-pages (https://miykael.github.io/nipype).

3 outstanding issues

Command line interface

If nipype_cmd is the same as nipypecli, does this mean that this content can be deleted?

Saving Workflows and Nodes to a file (experimental)

As mentioned above, this content is only in a WIP notebook and therefore not visible on the tutorial homepage. The problem is that wf.export('special_workflow.py') currently leads to an error in the notebook. Should we leave this as a WIP or remove the content?

Resource Scheduling and Profiling with Nipype

The resource profiler content is only as a WIP in the tutorial and doesn't work in the notebook, yet. I think this is addressed in #2571, but the proposed solution doesn't work in the container.

Mishap

Also, I unfortunately first updated the gh-pages from nipy/nipype. I didn't had a gh-pages branch yet, and so git checkout gh-pages put me directly on the upstream one... I'm really sorry for that and am happy to correct it, if necessary (gh-pages doesn't seem to be used anymore?). How can I revert my changes without bringing github down (again?)?

@effigies
Copy link
Member

I reverted gh-pages to Satra's latest commit. Fortunately you didn't destroy the history.

Yes, nipype_cmd no longer exists. I don't know whether that information can be (usefully) updated for nipypecli or if it just needs deleting. I'm content to leave that decision to you.

I would leave the WIP notebooks as WIP at least until 2.0. Perhaps someone will fix the issues? I would go ahead and open issues for those notebooks here, as the fixes probably need to be done here.

Happy to be overruled by @satra on any and all points.

@effigies effigies mentioned this pull request Oct 26, 2018
12 tasks
@effigies
Copy link
Member

@miykael Are you happy with this at least for this release? We can open issues for any remaining things to fix in 1.1.5.

@effigies effigies merged commit a2519fc into nipy:master Oct 31, 2018
@miykael
Copy link
Collaborator Author

miykael commented Oct 31, 2018

@effigies, yes. In my opinion it's good to go. Will keep track about any issues and open missing issues for the WIP on the tutrial homepage.

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.

3 participants