[ENH] Abandon distutils #1627

oesteban · 2016-09-13T16:44:04Z

This PR removes the dependency on distutils and uses only setuptools.

Following the recommendations I have reorganized the way nipype installs. Reading through this discussion it seems that requirements.txt should be written for development environments, and production (pypi, mostly) dependencies should be handled with setuptools.

Therefore, in this PR we start using all the available possibilities (setup_requires, install_requires, test_requires, extra_requires, etc.) to install nipype smoothly.

I am testing all the possibilities with fresh environments:

Regular user install through pypi, with the duecredit extra dependency:
```
pip install nipype[duecredit]
```
Regular user install from master:
```
python setup.py install
```

Developer install:

pip install -r requirements.txt
pip install -e .[test,doc]

Testing install:

pip install -r requirements.txt
pip install -e .[all]

The available extra dependencies are listed here: https://github.com/oesteban/nipype/blob/enh/MigrateToSetuptools/nipype/info.py#L157

This PR requires #1623 to be merged first. Closes #1605

oesteban · 2016-09-13T18:52:56Z

One possible improvement to help novice users would be adding a check on the available third party tools (AFNI, FSL, FreeSurfer, etc.) and report it ala matplotlib, warning the user about those missing software pieces.

codecov-io · 2016-09-13T20:24:37Z

Current coverage is 70.95% (diff: 100%)

Merging #1627 into master will increase coverage by 0.05%

@@             master      #1627   diff @@
==========================================
  Files          1027       1027          
  Lines         51574      51577     +3   
  Methods           0          0          
  Messages          0          0          
  Branches       7309       7310     +1   
==========================================
+ Hits          36564      36596    +32   
+ Misses        13917      13887    -30   
- Partials       1093       1094     +1

Powered by Codecov. Last update 5427538...8d28bde

satra · 2016-09-13T20:27:40Z

@oesteban - i would make that part of the new cli #1608 and it could internally use a function that checks and returns underlying tool status

oesteban · 2016-09-13T20:42:40Z

That sounds pretty cool!

Something like nipype check_deps ?

But I guess that'd be better in another PR, right?

…ptools

satra · 2016-10-01T20:28:32Z

@oesteban - resolve conflicts here.

satra · 2016-10-03T15:02:36Z

nipype/info.py

+EXTRA_REQUIRES = {
+    'doc': ['Sphinx>=0.3'],
+    'tests': TESTS_REQUIRES,
+    'fmri': ['nitime', 'nilearn'],


shouldn't this also have support for nipy and dipy and matplotlib?

the fmri extra or the tests extras? I guess so, this is the best moment to decide the dependencies and how we organize them

perhaps for both, but at present for the various interfaces.

Actually, for the doc extra, what are the dependencies?? There were some graphviz-related dependencies here right?. Also matplotlib.

Ok, found it. As per #1350, what do you think? pygraphviz or pydot (or pydot2 #1280) - @satra

all of them actually require graphviz installed. but if pydot2 or pydotplus works, i think those are the most recent things. i haven't looked into this in a while.

satra · 2016-10-03T15:03:06Z

nipype/info.py

@@ -70,27 +70,26 @@ def get_nipype_gitversion():
 # Note: this long_description is actually a copy/paste from the top-level
 # README.txt, so that it shows up nicely on PyPI.  So please remember to edit
 # it only in one place and sync it correctly.
-long_description = \
-    """
+long_description = """\


a triple-quoted string should not require a \

This is to avoid that newline, but I can remove it.

You can make the string start right after the quotes.

Ok, np. It just looked nicer to me this way. Probably not for others, will change XD

satra · 2016-10-03T15:03:52Z

@oesteban - should we merge this before or after the command line changes?

oesteban · 2016-10-03T15:40:55Z

@satra I'd do this before. When the command line can be added as an entry_point, which is a better way of adding console scripts.

oesteban · 2016-10-04T07:24:56Z

Ok, this is ready. However, please double check that writing the COMMIT_INFO still works. Other than that, I think this is pretty done

satra · 2016-10-04T13:17:07Z

@oesteban - still has byte/str issues:

on python 3, any stream from popen returns bytes and should be stripped and decoded.

In [2]: nipype.get_info()
Out[2]: 
{'commit_hash': "b'3e7dd7f\\n'",
 'commit_source': 'installation',
 'networkx_version': '1.11',
 'nibabel_version': '2.2.0dev',
 'numpy_version': '1.11.1',
 'pkg_path': '/software/anaconda/envs/dev3pype/lib/python3.5/site-packages/nipype-0.13.0_g3e7dd7f.dev-py3.5.egg/nipype',
 'scipy_version': '0.18.0',
 'sys_executable': '/software/anaconda/envs/dev3pype/bin/python',
 'sys_platform': 'darwin',
 'sys_version': '3.5.2 | packaged by conda-forge | (default, Jul 26 2016, 01:37:38) \n[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.54)]',
 'traits_version': '4.5.0'}

satra

this works fine in py2 but not in py3 because bytes are str are equivalent in py2 but different in py3.

satra · 2016-10-04T13:18:50Z

setup.py

+                                shell=True)
+        repo_commit, _ = proc.communicate()
+        # Fix for python 3
+        repo_commit = '{}'.format(repo_commit)


this should use repo_commit.decode() on py3

oesteban · 2016-10-04T16:38:26Z

@satra - I've been also looking at getting rid of that long list of packages, using find_packages() from setuptools:

psetup = find_packages(exclude=['*.tests'])

print '\n'.join(list(set(packages)-set(psetup)))
nipype.testing.data.tbss_dir
nipype.caching.tests
nipype.interfaces.semtools.brains.tests
nipype.interfaces.semtools.tests
nipype.interfaces.elastix.tests
nipype.interfaces.mrtrix.tests
nipype.workflows.fmri.fsl.tests
nipype.interfaces.dipy.tests
nipype.utils.tests
nipype.interfaces.ants.tests
nipype.interfaces.slicer.legacy.diffusion.tests
nipype.interfaces.vista.tests
nipype.testing.data.bedpostxout
nipype.interfaces.semtools.segmentation.tests
nipype.interfaces.script_templates
nipype.interfaces.semtools.diffusion.tests
nipype.interfaces.diffusion_toolkit.tests
nipype.algorithms.tests
nipype.interfaces.slicer.legacy.tests
nipype.interfaces.slicer.diffusion.tests
nipype.interfaces.nitime.tests
nipype.pipeline.plugins.tests
nipype.workflows.dmri.fsl.tests
nipype.interfaces.slicer.segmentation.tests
nipype.interfaces.freesurfer.tests
nipype.interfaces.camino2trackvis.tests
nipype.interfaces.slicer.quantification.tests
nipype.interfaces.tests
nipype.interfaces.camino.tests
nipype.interfaces.fsl.tests
nipype.interfaces.cmtk.tests
nipype.interfaces.semtools.filtering.tests
nipype.interfaces.spm.tests
nipype.interfaces.mipav.tests
nipype.interfaces.mne.tests
nipype.interfaces.nipy.tests
nipype.interfaces.mrtrix3.tests
nipype.interfaces.semtools.registration.tests
nipype.interfaces.minc.tests
nipype.interfaces.slicer.registration.tests
nipype.interfaces.afni.tests
nipype.interfaces.semtools.diffusion.tractography.tests
nipype.interfaces.semtools.legacy.tests
nipype.pipeline.engine.tests
nipype.testing.data.dicomdir
nipype.testing.data
nipype.interfaces.semtools.utilities.tests
nipype.interfaces.slicer.filtering.tests
nipype.interfaces.slicer.tests
nipype.workflows.fmri.spm.tests

Do we really want to install all those *.tests packages? Then there are other folders that I'm not sure they should be packages as well (nipype.testing.data.tbss_dir, nipype.testing.data.bedpostxout, nipype.testing.data, nipype.testing.data.dicomdir, nipype.interfaces.script_templates, etc.)

oesteban added 3 commits September 13, 2016 01:45

[ENH] Abandon distutils, only setuptools

1b4d4a3

fix python 3 install

2c3b7b0

fixing extras

3960782

satra added the in-progress label Sep 13, 2016

oesteban added needs-review 3 - Review labels Sep 13, 2016

remove future dependency from setup

db50359

oesteban added 2 commits September 14, 2016 23:14

Merge remote-tracking branch 'upstream/master' into enh/MigrateToSetu…

5573228

…ptools

Merge remote-tracking branch 'upstream/master' into enh/MigrateToSetu…

608dc5c

…ptools

oesteban mentioned this pull request Sep 16, 2016

[FIX] setup.py: remove future and configparse deps #1639

Merged

oesteban added 2 commits September 22, 2016 18:49

Merge remote-tracking branch 'upstream/master' into enh/MigrateToSetu…

a078fbe

…ptools

add nilearn to extra dependencies

b1aed64

oesteban added 2 commits October 2, 2016 19:15

Merge branch 'master' into enh/MigrateToSetuptools

1da2eb0

fix travis file

41972c6

satra reviewed Oct 3, 2016

View reviewed changes

update dependencies, minor fixes requested by satra

3e7dd7f

satra reviewed Oct 4, 2016

View reviewed changes

fix commit hash reading in PY3

5adbb88

use find_packages

8d28bde

oesteban mentioned this pull request Oct 4, 2016

WIP: Move nipype commands to a group command #1608

Merged

satra merged commit dbd53b2 into nipy:master Oct 4, 2016

satra removed the in-progress label Oct 4, 2016

oesteban deleted the enh/MigrateToSetuptools branch February 14, 2017 19:03

[ENH] Abandon distutils #1627

[ENH] Abandon distutils #1627

Uh oh!

Conversation

oesteban commented Sep 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oesteban commented Sep 13, 2016

Uh oh!

codecov-io commented Sep 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 70.95% (diff: 100%)

Uh oh!

satra commented Sep 13, 2016

Uh oh!

oesteban commented Sep 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satra commented Oct 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oesteban Oct 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

satra commented Oct 3, 2016

Uh oh!

oesteban commented Oct 3, 2016

Uh oh!

oesteban commented Oct 4, 2016

Uh oh!

satra commented Oct 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oesteban commented Oct 4, 2016

Uh oh!

Uh oh!

oesteban commented Sep 13, 2016 •

edited

Loading

codecov-io commented Sep 13, 2016 •

edited

Loading

oesteban commented Sep 13, 2016 •

edited

Loading

oesteban Oct 3, 2016 •

edited

Loading

satra commented Oct 4, 2016 •

edited

Loading