Skip to content

DOCS: add brief neurodocker tutorial #2464

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
Feb 24, 2018
Merged

Conversation

kaczmarj
Copy link
Collaborator

Fixes #2439.

Changes proposed in this pull request

@mgxd mgxd added this to the 1.0.1 milestone Feb 23, 2018
@@ -14,7 +14,7 @@ You can follow the `Nipype tutorial <https://miykael.github.io/nipype_tutorial/>
or use this docker container: `docker pull nipype/nipype`

or if you want to build custom docker containers with specific versions of
software see `Neurodocker <https://github.com/kaczmarj/neurodocker>`_
software see Neurodocker_. Read the `brief tutorial <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.

@kaczmarj - i don't think the html is necessary.

just: brief tutorial <neurodocker_tutorial>_.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@satra - i could not get that to render properly, so i used :doc:neurodocker instead.

@@ -108,7 +108,7 @@ Interface Dependencies
Nipype provides wrappers around many neuroimaging tools and contains some
algorithms. These tools will need to be installed for Nipype to run. You can
create containers with different versions of these tools installed using
`Neurodocker <https://github.com/kaczmarj/neurodocker>`_
Neurodocker_. Read the `brief tutorial <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.

same here

Copy link
Member

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM - minor comments

image. The package manager is ``apt`` or ``yum``, depending on the base
image.
2. Next, users should configure the container to fit their needs. This includes
installing neuroimaing software, installing packages from the chosen package
Copy link
Member

Choose a reason for hiding this comment

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

neuroimaging

Use NeuroDebian
---------------

This example install AFNI and ANTs from the NeuoDebian repositories. It also
Copy link
Member

Choose a reason for hiding this comment

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

installs

This example install AFNI and ANTs from the NeuoDebian repositories. It also
installs ``git`` and ``vim``.
::
$ docker run --rm kaczmarj/neurodocker:v0.3.2 generate --base neurodebian:stretch --pkg-manager apt --install afni ants git vim
Copy link
Member

Choose a reason for hiding this comment

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

perhaps break this line with a backslash

@djarecka
Copy link
Collaborator

@kaczmarj - don't forget to add the new tutorial to index.rst

@djarecka
Copy link
Collaborator

just noticed that this is part of install.rst, so ignore my previous comment

@effigies
Copy link
Member

effigies commented Feb 23, 2018 via email

@codecov-io
Copy link

codecov-io commented Feb 23, 2018

Codecov Report

Merging #2464 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2464      +/-   ##
==========================================
- Coverage   66.67%   66.66%   -0.01%     
==========================================
  Files         328      328              
  Lines       42551    42551              
  Branches     5276     5276              
==========================================
- Hits        28370    28366       -4     
- Misses      13501    13505       +4     
  Partials      680      680
Flag Coverage Δ
#smoketests 50.76% <ø> (ø) ⬆️
#unittests 63.98% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/plugins/multiproc.py 79.76% <0%> (-2.39%) ⬇️

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 1ecd0ff...25697ad. Read the comment docs.

@kaczmarj
Copy link
Collaborator Author

Should I update the links to the tutorial? I got the syntax from https://github.com/nipy/nipype/blob/master/doc/users/joinnode_and_itersource.rst.

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.

A couple stylistic comments. Overall this looks great.

@@ -14,7 +14,7 @@ You can follow the `Nipype tutorial <https://miykael.github.io/nipype_tutorial/>
or use this docker container: `docker pull nipype/nipype`

or if you want to build custom docker containers with specific versions of
software see `Neurodocker <https://github.com/kaczmarj/neurodocker>`_
software see Neurodocker_. Read the brief :doc:`neurodocker`.
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the way this whole section ends up looking, with blank lines followed by sentence fragments.

What about:

To get started using Docker, you can follow the `Nipype tutorial
<https://miykael.github.io/nipype_tutorial/>`_, or pull the `nipype/nipype`
image from Docker hub::

    docker pull nipype/nipype

You may also build custom docker containers with specific versions of software
using Neurodocker_ (see `Neurodocker tutorial`_).

(:doc: works, so go ahead and do that if you want. I'm just demonstrating what I meant earlier.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this, but the Neurodocker tutorial link still does not render properly for me when written this way. I kept the :doc: syntax.

`Neurodocker <https://github.com/kaczmarj/neurodocker>`_
Neurodocker_. Read the brief :doc:`neurodocker`.


Copy link
Member

Choose a reason for hiding this comment

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

Too many blank lines.

@@ -108,7 +108,9 @@ Interface Dependencies
Nipype provides wrappers around many neuroimaging tools and contains some
algorithms. These tools will need to be installed for Nipype to run. You can
create containers with different versions of these tools installed using
`Neurodocker <https://github.com/kaczmarj/neurodocker>`_
Neurodocker_. Read the brief :doc:`neurodocker`.
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that this be parenthetical, rather than a sentence. See above suggestion.

@effigies effigies merged commit 81bcf41 into nipy:master Feb 24, 2018
@kaczmarj kaczmarj deleted the docs/neurodocker branch February 26, 2018 14:39
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.

6 participants