From 595a70d20f09e3de1ac930ef9c9f449b609db5cd Mon Sep 17 00:00:00 2001 From: Dorota Jarecka Date: Fri, 2 Mar 2018 21:15:50 -0500 Subject: [PATCH 1/6] updating testing doc and a few changes to contributing.md --- CONTRIBUTING.md | 49 +++++++++++++--------- doc/devel/testing_nipype.rst | 81 +++++++++++------------------------- 2 files changed, 55 insertions(+), 75 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a6b91a52ab..d318b2d5bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,9 @@ Welcome to the Nipype repository! We're excited you're here and want to contribu 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]! -Before you start you'll need to set up a free [GitHub][link_github] account and sign in. Here are some [instructions][link_signupinstructions]. +Before you start you'll need to set up a free [GitHub][link_github] account and sign in, here are some [instructions][link_signupinstructions]. +If you are not familiar with version control system and Git, +you will find introduction and links to tutorials [here](http://www.reproducibleimaging.org/module-reproducible-basics/02-vcs/). Already know what you're looking for in this guide? Jump to the following sections: * [Understanding issue labels](#issue-labels) @@ -47,30 +49,35 @@ This allows other members of the Nipype development team to confirm that you are **2. [Fork][link_fork] the [Nipype repository][link_nipype] to your profile.** -This is now your own unique copy of Nipype. +This is now your own unique copy of Nipype repository. Changes here won't affect anyone else's work, so it's a safe space to explore edits to the code! -Make sure to [keep your fork up to date][link_updateupstreamwiki] with the master repository. +You can clone your Nipype repository in order to create a local copy of the code on your machine. +In your local Nipype directory, run `pip install -e .[dev]`. +This will add your version of nipype to your local python environment and +install dependencies needed for development. -**3. Make the changes you've discussed.** +Make sure to [keep your fork up to date][link_updateupstreamwiki] with the original Nipype repository. -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]. +**3. Make the changes you've discussed.** -To confirm that your changes worked as intended, [clone your fork][link_cloning] to create a local directory. -In this local directory, run `python setup.py develop`. -This will add your version of nipype to your local python environment. +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]. -Then, in this local nipype directory, run `make check-before-commit`. If you get no errors, you're ready to submit your changes! +When you are working on your changes, you should test frequently if you are not breaking the existing code, +more on testing you will find [the testing section of Nipype documentation](http://nipype.readthedocs.io/en/latest/devel/testing_nipype.html). -**4. Submit a [pull request][link_pullrequest].** +It's a good practice to create a new branch of your Nipype repository for a new set of changes. -Submit a new pull request for your changes, using the tags outlined in the [tagging pull requests section](#tagging-pull-requests). +Before pushing your changes to GitHub run `make check-before-commit`, this will remove trailing spaces, create new auto tests, +test entire package and build the documentation. +If you get no errors, you're ready to submit your changes! -A member of the development team will review your changes to confirm that they can be merged into the main codebase. +**4. Submit a [pull request][link_pullrequest].** -## Tagging Pull Requests +A new pull request for your changes should be created from your GitHub account. -Pull requests should be submitted early and often! When opening a pull request, please use one of the following prefixes: +When opening a pull request, please use one of the following prefixes: * **[ENH]** for enhancements @@ -81,11 +88,14 @@ Pull requests should be submitted early and often! When opening a pull request, * **[REF]** for refactoring existing code
- -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. +Pull requests should be submitted early and often (please don't mix too many unrelated changes within one PR)! +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). +This tells the development team that your pull request is a "work-in-progress", and that you plan to continue working on it. Review and discussion on new code can begin well before the work is complete, and the more discussion the better! -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: +This provides the opportunity to check with the development team the path you've outlined. + +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. ## Notes for New Code @@ -98,8 +108,9 @@ Do not log this, as it creates redundant/confusing logs. #### Testing New code should be tested, whenever feasible. -Bug fixes should include regression tests, and any new behavior should at least get minimal exercise. -If you're not sure what this means for your code, ask! +Bug fixes should include an example that exposes the issue. +Any new features should have tests that show at least a minimal example. +If you're not sure what this means for your code, please ask in your pull request. ## Recognizing contributions diff --git a/doc/devel/testing_nipype.rst b/doc/devel/testing_nipype.rst index ff1a14fe6c..8d04be215e 100644 --- a/doc/devel/testing_nipype.rst +++ b/doc/devel/testing_nipype.rst @@ -27,30 +27,18 @@ After cloning:: cd nipype pip install -r requirements.txt - python setup.py develop - -or:: - - cd nipype - pip install -r requirements.txt - pip install -e .[tests] - + pip install -e .[dev] Test implementation ------------------- Nipype testing framework is built upon `pytest `_. -By the time these guidelines are written, Nipype implements 17638 tests. After installation in developer mode, the tests can be run with the -following simple command at the root folder of the project :: - - make tests +following command at the root folder of the project :: -If ``make`` is not installed in the system, it is possible to run the tests using:: - - py.test --doctest-modules --cov=nipype nipype + pytest -v --doctest-modules nipype A successful test run should complete in 10-30 minutes and end with @@ -75,69 +63,50 @@ where ``$pathtomatlabdir`` is the path to your matlab installation and ``$platform`` is the directory referring to x86 or x64 installations (typically ``glnxa64`` on 64-bit installations). -Skip tests +Skipped tests ~~~~~~~~~~ Nipype will skip some tests depending on the currently available software and data dependencies. Installing software dependencies and downloading the necessary data -will reduce the number of skip tests. +will reduce the number of skipped tests. -Some tests in Nipype make use of some images distributed within the `FSL course data +A few tests in Nipype make use of some images distributed within the `FSL course data `_. This reduced version of the package can be downloaded `here `_. To enable the tests depending on these data, just unpack the targz file and set the :code:`FSL_COURSE_DATA` environment -variable to point to that folder. +variable to point to that folder. +Note, that the test execution time can increase significantly with these additional tests. + -Xfail tests +Xfailed tests ~~~~~~~~~~~ Some tests are expect to fail until the code will be changed or for other reasons. -Avoiding any MATLAB calls from testing -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -On unix systems, set an empty environment variable:: - - export NIPYPE_NO_MATLAB= - -This will skip any tests that require matlab. - - Testing Nipype using Docker --------------------------- -As of :code:`nipype-0.13`, Nipype is tested inside Docker containers. First, install the -`Docker Engine `_. -Nipype has one base docker image called nipype/base:latest, and several additional test images -for various Python versions. - -The base nipype image is built as follows:: +Nipype is tested inside Docker containers and users can use nipype images to test local versions. +First, install the `Docker Engine `_. +Nipype has one base docker image called nipype/base:latest, that contains several useful tools +(FreeSurfer, AFNI, FSL, ANTs, etc.), and additional test images +for specific Python versions: py27 for Python 2.7 and py36 for Python 3.6. - cd path/to/nipype/ - docker build -t nipype/base:latest -f docker/base.Dockerfile . +Users can pull the nipype image for Python 3.6 as follows:: + + docker pull nipype/base:py36 -This base image contains several useful tools (FreeSurfer, AFNI, FSL, ANTs, etc.), -but not nipype. +In order to test a local version of nipype you can run test within container as follows:: -It is possible to fetch a built image from the latest master branch of nipype -using:: + docker run -it -v $PWD:/src/nipype --rm nipype/nipype:py36 py.test -v --doctest-modules /src/nipype/nipype - docker run -it --rm nipype/nipype:master +Additional comments +------------------- -The docker run command will then open the container and offer a bash shell for the -developer. - -For building a continer for running nipype in Python 3.6:: +If the project is tested both on your local OS and within a Docker container you might have to remove all +``__pycache__`` directory before changing between system. - cd path/to/nipype/ - docker build -f Dockerfile -t nipype/nipype_test:py36 . - docker run -it --rm -e FSL_COURSE_DATA="/root/examples/nipype-fsl_course_data" \ - -v ~/examples:/root/examples:ro \ - -v ~/scratch:/scratch \ - -w /root/src/nipype \ - nipype/nipype_test:py36 /usr/bin/run_pytests.sh -The last examples assume that the example data is downladed into ~/examples and -the ~/scratch folder will be created if it does not exist previously. + From f5ed23cf5042875bc938d0bfc664c026d5060568 Mon Sep 17 00:00:00 2001 From: Dorota Jarecka Date: Fri, 2 Mar 2018 21:25:45 -0500 Subject: [PATCH 2/6] adding links --- CONTRIBUTING.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d318b2d5bf..567f938d45 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,15 +67,17 @@ check out the [guide for adding new interfaces to Nipype][link_new_interfaces]. When you are working on your changes, you should test frequently if you are not breaking the existing code, more on testing you will find [the testing section of Nipype documentation](http://nipype.readthedocs.io/en/latest/devel/testing_nipype.html). -It's a good practice to create a new branch of your Nipype repository for a new set of changes. - Before pushing your changes to GitHub run `make check-before-commit`, this will remove trailing spaces, create new auto tests, test entire package and build the documentation. If you get no errors, you're ready to submit your changes! +It's a good practice to create [a new branch](https://help.github.com/articles/about-branches/) +of the repository for a new set of changes. + + **4. Submit a [pull request][link_pullrequest].** -A new pull request for your changes should be created from your GitHub account. +A new pull request for your changes should be [created from your GitHub account](https://help.github.com/articles/creating-a-pull-request-from-a-fork/). When opening a pull request, please use one of the following prefixes: From 9803db73652905b47343e77e10edb9d71204e009 Mon Sep 17 00:00:00 2001 From: Dorota Jarecka Date: Fri, 2 Mar 2018 21:28:36 -0500 Subject: [PATCH 3/6] changing a link about PR --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 567f938d45..a206caeb16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -77,7 +77,7 @@ of the repository for a new set of changes. **4. Submit a [pull request][link_pullrequest].** -A new pull request for your changes should be [created from your GitHub account](https://help.github.com/articles/creating-a-pull-request-from-a-fork/). +A new pull request for your changes should be created from your fork of the repository. When opening a pull request, please use one of the following prefixes: @@ -145,7 +145,7 @@ You're awesome. :wave::smiley: [link_good_first_issue]: https://github.com/nipy/nipype/issues?q=is%3Aopen+is%3Aissue+label%3Agood-first-issue [link_enhancement]: https://github.com/nipy/nipype/labels/enhancement -[link_pullrequest]: https://help.github.com/articles/creating-a-pull-request/ +[link_pullrequest]: https://help.github.com/articles/creating-a-pull-request-from-a-fork/ [link_fork]: https://help.github.com/articles/fork-a-repo/ [link_pushpullblog]: https://www.igvita.com/2011/12/19/dont-push-your-pull-requests/ [link_updateupstreamwiki]: https://help.github.com/articles/syncing-a-fork/ From 7ea352576bb51adea0b30a76e80ec8de26842147 Mon Sep 17 00:00:00 2001 From: Dorota Jarecka Date: Mon, 5 Mar 2018 14:15:25 -0500 Subject: [PATCH 4/6] suggestions from Satra --- doc/devel/testing_nipype.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/devel/testing_nipype.rst b/doc/devel/testing_nipype.rst index 8d04be215e..326caeaea4 100644 --- a/doc/devel/testing_nipype.rst +++ b/doc/devel/testing_nipype.rst @@ -64,7 +64,7 @@ where ``$pathtomatlabdir`` is the path to your matlab installation and (typically ``glnxa64`` on 64-bit installations). Skipped tests -~~~~~~~~~~ +~~~~~~~~~~~~~ Nipype will skip some tests depending on the currently available software and data dependencies. Installing software dependencies and downloading the necessary data @@ -79,7 +79,7 @@ Note, that the test execution time can increase significantly with these additio Xfailed tests -~~~~~~~~~~~ +~~~~~~~~~~~~~ Some tests are expect to fail until the code will be changed or for other reasons. @@ -106,7 +106,7 @@ Additional comments ------------------- If the project is tested both on your local OS and within a Docker container you might have to remove all -``__pycache__`` directory before changing between system. +``__pycache__`` directories before switching between your OS and a container. From 86ecf337ddba45a421b73e351cbf80e5cbb91bf9 Mon Sep 17 00:00:00 2001 From: Dorota Jarecka Date: Wed, 7 Mar 2018 13:17:32 -0500 Subject: [PATCH 5/6] suggestions from Chris M. --- CONTRIBUTING.md | 29 ++++++++++++++++------------- doc/devel/testing_nipype.rst | 13 +++++-------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a206caeb16..972951c827 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,9 +4,10 @@ Welcome to the Nipype repository! We're excited you're here and want to contribu 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]! -Before you start you'll need to set up a free [GitHub][link_github] account and sign in, here are some [instructions][link_signupinstructions]. -If you are not familiar with version control system and Git, -you will find introduction and links to tutorials [here](http://www.reproducibleimaging.org/module-reproducible-basics/02-vcs/). +Before you start you'll need to set up a free [GitHub][link_github] account and sign in. Here are some [instructions][link_signupinstructions]. +If you are not familiar with version control systems such as git, + [introductions and tutorials](http://www.reproducibleimaging.org/module-reproducible-basics/02-vcs/) + may be found on [ReproducibleImaging.org](https://www.reproducibleimaging.org/). Already know what you're looking for in this guide? Jump to the following sections: * [Understanding issue labels](#issue-labels) @@ -49,26 +50,28 @@ This allows other members of the Nipype development team to confirm that you are **2. [Fork][link_fork] the [Nipype repository][link_nipype] to your profile.** -This is now your own unique copy of Nipype repository. +This is now your own unique copy of the Nipype repository. Changes here won't affect anyone else's work, so it's a safe space to explore edits to the code! You can clone your Nipype repository in order to create a local copy of the code on your machine. -In your local Nipype directory, run `pip install -e .[dev]`. -This will add your version of nipype to your local python environment and -install dependencies needed for development. +To install your version of Nipype, and the dependencies needed for development, +in your Python environment, run `pip install -e ".[dev]"` from your local Nipype +directory. -Make sure to [keep your fork up to date][link_updateupstreamwiki] with the original Nipype repository. +Make sure to keep your fork up to date with the original Nipype repository. +One way to do this is to [configure a new remote named "upstream"](https://help.github.com/articles/configuring-a-remote-for-a-fork/) + and to [sync your fork with the upstream repository][link_updateupstreamwiki]. **3. Make the changes you've discussed.** 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]. -When you are working on your changes, you should test frequently if you are not breaking the existing code, -more on testing you will find [the testing section of Nipype documentation](http://nipype.readthedocs.io/en/latest/devel/testing_nipype.html). +When you are working on your changes, test frequently to ensure you are not breaking the existing code. +For more on testing, please see [the testing section of Nipype documentation](http://nipype.readthedocs.io/en/latest/devel/testing_nipype.html). -Before pushing your changes to GitHub run `make check-before-commit`, this will remove trailing spaces, create new auto tests, -test entire package and build the documentation. +Before pushing your changes to GitHub, run `make check-before-commit`. This will remove trailing spaces, create new auto tests, +test the entire package, and build the documentation. If you get no errors, you're ready to submit your changes! It's a good practice to create [a new branch](https://help.github.com/articles/about-branches/) @@ -95,7 +98,7 @@ If your pull request is not yet ready to be merged, please also include the **[W This tells the development team that your pull request is a "work-in-progress", and that you plan to continue working on it. Review and discussion on new code can begin well before the work is complete, and the more discussion the better! -This provides the opportunity to check with the development team the path you've outlined. +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. 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. diff --git a/doc/devel/testing_nipype.rst b/doc/devel/testing_nipype.rst index 326caeaea4..dc819a63b8 100644 --- a/doc/devel/testing_nipype.rst +++ b/doc/devel/testing_nipype.rst @@ -89,13 +89,13 @@ Testing Nipype using Docker Nipype is tested inside Docker containers and users can use nipype images to test local versions. First, install the `Docker Engine `_. -Nipype has one base docker image called nipype/base:latest, that contains several useful tools -(FreeSurfer, AFNI, FSL, ANTs, etc.), and additional test images +Nipype has one base docker image called nipype/nipype:base, that contains several useful tools + (FreeSurfer, AFNI, FSL, ANTs, etc.), and additional test images for specific Python versions: py27 for Python 2.7 and py36 for Python 3.6. Users can pull the nipype image for Python 3.6 as follows:: - docker pull nipype/base:py36 + docker pull nipype/nipype:py36 In order to test a local version of nipype you can run test within container as follows:: @@ -105,8 +105,5 @@ In order to test a local version of nipype you can run test within container as Additional comments ------------------- -If the project is tested both on your local OS and within a Docker container you might have to remove all -``__pycache__`` directories before switching between your OS and a container. - - - +If the project is tested both on your local OS and within a Docker container, you might have to remove all +``__pycache__`` directories before switching between your OS and a container. \ No newline at end of file From 201a2efaef825002c8321e74a8909e6805fa6bce Mon Sep 17 00:00:00 2001 From: Satrajit Ghosh Date: Fri, 9 Mar 2018 10:17:09 -0500 Subject: [PATCH 6/6] sty: add clarity for debian --- doc/devel/testing_nipype.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/devel/testing_nipype.rst b/doc/devel/testing_nipype.rst index dc819a63b8..03d063f2e5 100644 --- a/doc/devel/testing_nipype.rst +++ b/doc/devel/testing_nipype.rst @@ -54,8 +54,8 @@ environment variable is not set, some FreeSurfer related tests will fail. If any of the tests failed, please report them on our `bug tracker `_. -On Debian systems, set the following environment variable before running -tests:: +On Debian systems with a local copy of MATLAB installed, set the following +environment variable before running tests:: export MATLABCMD=$pathtomatlabdir/bin/$platform/MATLAB @@ -106,4 +106,4 @@ Additional comments ------------------- If the project is tested both on your local OS and within a Docker container, you might have to remove all -``__pycache__`` directories before switching between your OS and a container. \ No newline at end of file +``__pycache__`` directories before switching between your OS and a container.