Skip to content

Commit 04b440a

Browse files
authored
Merge pull request #2482 from djarecka/updating_contributing_2445
Updating contributing and testing (closes #2445)
2 parents 04aa3c5 + 201a2ef commit 04b440a

File tree

2 files changed

+62
-81
lines changed

2 files changed

+62
-81
lines changed

CONTRIBUTING.md

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ Welcome to the Nipype repository! We're excited you're here and want to contribu
55
These guidelines are designed to make it as easy as possible to get involved. If you have any questions that aren't discussed below, please let us know by opening an [issue][link_issues]!
66

77
Before you start you'll need to set up a free [GitHub][link_github] account and sign in. Here are some [instructions][link_signupinstructions].
8+
If you are not familiar with version control systems such as git,
9+
[introductions and tutorials](http://www.reproducibleimaging.org/module-reproducible-basics/02-vcs/)
10+
may be found on [ReproducibleImaging.org](https://www.reproducibleimaging.org/).
811

912
Already know what you're looking for in this guide? Jump to the following sections:
1013
* [Understanding issue labels](#issue-labels)
@@ -55,31 +58,39 @@ This allows other members of the Nipype development team to confirm that you are
5558

5659
**2. [Fork][link_fork] the [Nipype repository][link_nipype] to your profile.**
5760

58-
This is now your own unique copy of Nipype.
61+
This is now your own unique copy of the Nipype repository.
5962
Changes here won't affect anyone else's work, so it's a safe space to explore edits to the code!
6063

61-
Make sure to [keep your fork up to date][link_updateupstreamwiki] with the master repository.
64+
You can clone your Nipype repository in order to create a local copy of the code on your machine.
65+
To install your version of Nipype, and the dependencies needed for development,
66+
in your Python environment, run `pip install -e ".[dev]"` from your local Nipype
67+
directory.
68+
69+
Make sure to keep your fork up to date with the original Nipype repository.
70+
One way to do this is to [configure a new remote named "upstream"](https://help.github.com/articles/configuring-a-remote-for-a-fork/)
71+
and to [sync your fork with the upstream repository][link_updateupstreamwiki].
6272

6373
**3. Make the changes you've discussed.**
6474

65-
If you're adding a new tool from an existing neuroimaging toolkit (e.g., 3dDeconvolve from AFNI), check out the [guide for adding new interfaces to Nipype][link_new_interfaces].
75+
If you're adding a new tool from an existing neuroimaging toolkit (e.g., 3dDeconvolve from AFNI),
76+
check out the [guide for adding new interfaces to Nipype][link_new_interfaces].
6677

67-
To confirm that your changes worked as intended, [clone your fork][link_cloning] to create a local directory.
68-
In this local directory, run `pip install -e .[dev]`.
69-
This will add your version of nipype to your local python environment and
70-
install dependencies needed for development.
78+
When you are working on your changes, test frequently to ensure you are not breaking the existing code.
79+
For more on testing, please see [the testing section of Nipype documentation](http://nipype.readthedocs.io/en/latest/devel/testing_nipype.html).
7180

72-
Then, in this local nipype directory, run `make check-before-commit`. If you get no errors, you're ready to submit your changes!
81+
Before pushing your changes to GitHub, run `make check-before-commit`. This will remove trailing spaces, create new auto tests,
82+
test the entire package, and build the documentation.
83+
If you get no errors, you're ready to submit your changes!
7384

74-
**4. Submit a [pull request][link_pullrequest].**
85+
It's a good practice to create [a new branch](https://help.github.com/articles/about-branches/)
86+
of the repository for a new set of changes.
7587

76-
Submit a new pull request for your changes, using the tags outlined in the [tagging pull requests section](#tagging-pull-requests).
7788

78-
A member of the development team will review your changes to confirm that they can be merged into the main codebase.
89+
**4. Submit a [pull request][link_pullrequest].**
7990

80-
## Tagging Pull Requests
91+
A new pull request for your changes should be created from your fork of the repository.
8192

82-
Pull requests should be submitted early and often! When opening a pull request, please use one of the following prefixes:
93+
When opening a pull request, please use one of the following prefixes:
8394

8495

8596
* **[ENH]** for enhancements
@@ -90,11 +101,14 @@ Pull requests should be submitted early and often! When opening a pull request,
90101
* **[REF]** for refactoring existing code
91102

92103
<br>
93-
94-
If, when you submit, your pull request is not yet ready to be merged, please also include the **[WIP]** prefix. This tells the development team that your pull request is a "work-in-progress", and that you plan to continue working on it.
104+
Pull requests should be submitted early and often (please don't mix too many unrelated changes within one PR)!
105+
If your pull request is not yet ready to be merged, please also include the **[WIP]** prefix (you can remove it once your PR is ready to be merged).
106+
This tells the development team that your pull request is a "work-in-progress", and that you plan to continue working on it.
95107

96108
Review and discussion on new code can begin well before the work is complete, and the more discussion the better!
97-
In the worst case scenario, if the development team decides to pursue a different path than you've outlined, they'll close the pull request. That's really not so bad! :smile:
109+
The development team may prefer a different path than you've outlined, so it's better to discuss it and get approval at the early stage of your work.
110+
111+
One your PR is ready a member of the development team will review your changes to confirm that they can be merged into the main codebase.
98112

99113
## Notes for New Code
100114

@@ -107,8 +121,9 @@ Do not log this, as it creates redundant/confusing logs.
107121

108122
#### Testing
109123
New code should be tested, whenever feasible.
110-
Bug fixes should include regression tests, and any new behavior should at least get minimal exercise.
111-
If you're not sure what this means for your code, ask!
124+
Bug fixes should include an example that exposes the issue.
125+
Any new features should have tests that show at least a minimal example.
126+
If you're not sure what this means for your code, please ask in your pull request.
112127

113128
## Recognizing contributions
114129

@@ -142,7 +157,7 @@ You're awesome. :wave::smiley:
142157
[link_enhancement]: https://github.com/nipy/nipype/labels/enhancement
143158
[link_orphaned]: https://github.com/nipy/nipype/pulls?q=is%3Apr+label%3Aorphaned+is%3Aclosed
144159

145-
[link_pullrequest]: https://help.github.com/articles/creating-a-pull-request/
160+
[link_pullrequest]: https://help.github.com/articles/creating-a-pull-request-from-a-fork/
146161
[link_fork]: https://help.github.com/articles/fork-a-repo/
147162
[link_pushpullblog]: https://www.igvita.com/2011/12/19/dont-push-your-pull-requests/
148163
[link_updateupstreamwiki]: https://help.github.com/articles/syncing-a-fork/

doc/devel/testing_nipype.rst

Lines changed: 28 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -27,30 +27,18 @@ After cloning::
2727

2828
cd nipype
2929
pip install -r requirements.txt
30-
python setup.py develop
31-
32-
or::
33-
34-
cd nipype
35-
pip install -r requirements.txt
36-
pip install -e .[tests]
37-
30+
pip install -e .[dev]
3831

3932

4033
Test implementation
4134
-------------------
4235

4336
Nipype testing framework is built upon `pytest <http://doc.pytest.org/en/latest/>`_.
44-
By the time these guidelines are written, Nipype implements 17638 tests.
4537

4638
After installation in developer mode, the tests can be run with the
47-
following simple command at the root folder of the project ::
39+
following command at the root folder of the project ::
4840

49-
make tests
50-
51-
If ``make`` is not installed in the system, it is possible to run the tests using::
52-
53-
py.test --doctest-modules --cov=nipype nipype
41+
pytest -v --doctest-modules nipype
5442

5543

5644
A successful test run should complete in 10-30 minutes and end with
@@ -66,78 +54,56 @@ environment variable is not set, some FreeSurfer related tests will fail.
6654
If any of the tests failed, please report them on our `bug tracker
6755
<http://github.com/nipy/nipype/issues>`_.
6856

69-
On Debian systems, set the following environment variable before running
70-
tests::
57+
On Debian systems with a local copy of MATLAB installed, set the following
58+
environment variable before running tests::
7159

7260
export MATLABCMD=$pathtomatlabdir/bin/$platform/MATLAB
7361

7462
where ``$pathtomatlabdir`` is the path to your matlab installation and
7563
``$platform`` is the directory referring to x86 or x64 installations
7664
(typically ``glnxa64`` on 64-bit installations).
7765

78-
Skip tests
79-
~~~~~~~~~~
66+
Skipped tests
67+
~~~~~~~~~~~~~
8068

8169
Nipype will skip some tests depending on the currently available software and data
8270
dependencies. Installing software dependencies and downloading the necessary data
83-
will reduce the number of skip tests.
71+
will reduce the number of skipped tests.
8472

85-
Some tests in Nipype make use of some images distributed within the `FSL course data
73+
A few tests in Nipype make use of some images distributed within the `FSL course data
8674
<http://fsl.fmrib.ox.ac.uk/fslcourse/>`_. This reduced version of the package can be downloaded `here
8775
<https://files.osf.io/v1/resources/nefdp/providers/osfstorage/57f472cf9ad5a101f977ecfe>`_.
8876
To enable the tests depending on these data, just unpack the targz file and set the :code:`FSL_COURSE_DATA` environment
89-
variable to point to that folder.
90-
91-
Xfail tests
92-
~~~~~~~~~~~
93-
94-
Some tests are expect to fail until the code will be changed or for other reasons.
95-
96-
97-
Avoiding any MATLAB calls from testing
98-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
77+
variable to point to that folder.
78+
Note, that the test execution time can increase significantly with these additional tests.
9979

100-
On unix systems, set an empty environment variable::
10180

102-
export NIPYPE_NO_MATLAB=
81+
Xfailed tests
82+
~~~~~~~~~~~~~
10383

104-
This will skip any tests that require matlab.
84+
Some tests are expect to fail until the code will be changed or for other reasons.
10585

10686

10787
Testing Nipype using Docker
10888
---------------------------
10989

110-
As of :code:`nipype-0.13`, Nipype is tested inside Docker containers. First, install the
111-
`Docker Engine <https://docs.docker.com/engine/installation/>`_.
112-
Nipype has one base docker image called nipype/base:latest, and several additional test images
113-
for various Python versions.
114-
115-
The base nipype image is built as follows::
90+
Nipype is tested inside Docker containers and users can use nipype images to test local versions.
91+
First, install the `Docker Engine <https://docs.docker.com/engine/installation/>`_.
92+
Nipype has one base docker image called nipype/nipype:base, that contains several useful tools
93+
(FreeSurfer, AFNI, FSL, ANTs, etc.), and additional test images
94+
for specific Python versions: py27 for Python 2.7 and py36 for Python 3.6.
11695

117-
cd path/to/nipype/
118-
docker build -t nipype/base:latest -f docker/base.Dockerfile .
96+
Users can pull the nipype image for Python 3.6 as follows::
97+
98+
docker pull nipype/nipype:py36
11999

120-
This base image contains several useful tools (FreeSurfer, AFNI, FSL, ANTs, etc.),
121-
but not nipype.
100+
In order to test a local version of nipype you can run test within container as follows::
122101

123-
It is possible to fetch a built image from the latest master branch of nipype
124-
using::
102+
docker run -it -v $PWD:/src/nipype --rm nipype/nipype:py36 py.test -v --doctest-modules /src/nipype/nipype
125103

126-
docker run -it --rm nipype/nipype:master
127104

105+
Additional comments
106+
-------------------
128107

129-
The docker run command will then open the container and offer a bash shell for the
130-
developer.
131-
132-
For building a continer for running nipype in Python 3.6::
133-
134-
cd path/to/nipype/
135-
docker build -f Dockerfile -t nipype/nipype_test:py36 .
136-
docker run -it --rm -e FSL_COURSE_DATA="/root/examples/nipype-fsl_course_data" \
137-
-v ~/examples:/root/examples:ro \
138-
-v ~/scratch:/scratch \
139-
-w /root/src/nipype \
140-
nipype/nipype_test:py36 /usr/bin/run_pytests.sh
141-
142-
The last examples assume that the example data is downladed into ~/examples and
143-
the ~/scratch folder will be created if it does not exist previously.
108+
If the project is tested both on your local OS and within a Docker container, you might have to remove all
109+
``__pycache__`` directories before switching between your OS and a container.

0 commit comments

Comments
 (0)