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

Remove GPL from our README. #1315

Merged
merged 7 commits into from Sep 26, 2017

Conversation

Projects
None yet
7 participants
@arokem
Member

arokem commented Aug 4, 2017

Also, added a few more small things + links.

Follow up to #1285

README.rst Outdated
Installing DIPY
===============
For installation instructions, please read our `documentation <http://nipy.org/dipy/installation.html>`_.

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 4, 2017

Member

I think we should start with
conda install -c conda-forge dipy vtk
and then provide the link to the rest of the installation information.

DIPY uses other libraries also licensed under the BSD or the

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 4, 2017

Member

Feels great! :)

@codecov-io

This comment has been minimized.

codecov-io commented Aug 4, 2017

Codecov Report

Merging #1315 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1315   +/-   ##
=======================================
  Coverage   87.02%   87.02%           
=======================================
  Files         228      228           
  Lines       28841    28841           
  Branches     3100     3100           
=======================================
  Hits        25099    25099           
  Misses       3037     3037           
  Partials      705      705

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 d737a6a...c381fcd. Read the comment docs.

@arokem

This comment has been minimized.

Member

arokem commented Aug 4, 2017

@coveralls

This comment has been minimized.

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 199f558 on arokem:no-gpl into ** on nipy:master**.

@coveralls

This comment has been minimized.

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling c52f857 on arokem:no-gpl into ** on nipy:master**.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling c52f857 on arokem:no-gpl into ** on nipy:master**.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

I would only keep conda in the Readme. Fast, easy and complete installation both api and viz. With pip things are a bit more complicated and there is no pip for vtk. Let's make our users happy. With one line you get it all :)

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

That keeps your users happy only if they are already using conda.

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

Ouch :)

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

@matthew-brett for the other options they can read the installation documentation.
I think Anaconda is extremely popular these days. But I am not sure what the actual stats are. I mean how many people are using anaconda vs other packaging methods. It seems to me that at least for scientific python - Anaconda is currently dominating and for many good reasons. Do you have a different understanding?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

Yes, I do - but even if I didn't, I don't think I'd try and force someone who was thus far using pip to use conda. There's a long discussion over on Scipy github as to why Anaconda monoculture might be undesirable.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Oops. Can you send me the link to the discussion?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

Here's one of Gael's comments in the discussion : scipy/scipy.org#179 (comment)

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Okay I see the issue @matthew-brett . The problem we have with pip is that there is no pip for vtk.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

And I actually do not understand why there is no pip for vtk. It would be wonderful to have that.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Do you know why?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

Yes, I do - I'm on the VTK mailing list and I was on a group hangout with the VTK guys about this a few weeks ago. The main problem is compiling a VTK wheel that can work with a reasonable number of backends. They are working on it at the moment, I guess that it will be done in a few months.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Cool, that is great. So, what do you suggest put pip in the readme now or wait for the VTK in pypi so that the information is more complete?

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 4, 2017

I suggest keeping pip in the readme with the caveat that the user should install VTK separately when using pip (via apt or homebrew). I think it's OK to recommend conda over pip, I'm just saying that you should keep in mind that a lot of people do use pip, and some people have a strong preference for pip.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Understood. We can add pip @arokem. Thanks @matthew-brett for pointing this out. +1

@coveralls

This comment has been minimized.

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 1d2dc73 on arokem:no-gpl into ** on nipy:master**.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Aug 4, 2017

Coverage Status

Changes Unknown when pulling 1d2dc73 on arokem:no-gpl into ** on nipy:master**.

@arokem

This comment has been minimized.

Member

arokem commented Aug 4, 2017

All good. I just reverted that last change, so we now have pip and conda.

@coveralls

This comment has been minimized.

coveralls commented Aug 5, 2017

Coverage Status

Changes Unknown when pulling 3f22fc7 on arokem:no-gpl into ** on nipy:master**.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Aug 5, 2017

Coverage Status

Changes Unknown when pulling 3f22fc7 on arokem:no-gpl into ** on nipy:master**.

@arokem

This comment has been minimized.

Member

arokem commented Aug 9, 2017

BTW -- still can't seem to install VTK on Python 3, even with conda. Added a comment about that.

@grlee77

This comment has been minimized.

Contributor

grlee77 commented Aug 9, 2017

BTW -- still can't seem to install VTK on Python 3, even with conda. Added a comment about that.

If you are on Windows or Linux, VTK 7.1.1 should be available for Python 2.7, 3.5 and 3.6 from conda-forge. If you are not on OS X, can you specify the problem you encountered with conda? We don't have OS X builds there due to build timeouts on the CI services.

p.s. You can see the specific package versions available at: https://anaconda.org/conda-forge/vtk/files

The conda-forge build script is compatible with OS X if you want to use it to build your own local package. I doubt you want to go into that level of detail in the Dipy docs, but if interested the procedure is:

1.) clone https://github.com/conda-forge/vtk-feedstock
2.) cd vtk-feedstock
3.) edit recipe/meta.yaml to remove the "skip" line so an OS X build won't be skipped.
4.) conda build -c conda-forge --python 3.6 recipe
5.) conda install --use-local vtk
@arokem

This comment has been minimized.

Member

arokem commented Aug 9, 2017

Thanks!

I see:

chlosyne:~  $conda install -c conda-forge vtk
Fetching package metadata ...........
Solving package specifications: .

UnsatisfiableError: The following specifications were found to be in conflict:
  - python 3.5*
  - vtk -> python 2.7*
Use "conda info <package>" to see the dependencies for each package.
@arokem

This comment has been minimized.

Member

arokem commented Aug 9, 2017

Should say: on OSX. I will try your more detailed instructions, but I definitely don't think that we want that level of detail in our README.

@arokem

This comment has been minimized.

Member

arokem commented Aug 9, 2017

I can't get the more detailed instructions you provided to work for me. I get:

chlosyne:vtk-feedstock (master) $conda build -c conda-forge --python 3.5 recipe
Traceback (most recent call last):
  File "/Users/arokem/anaconda3/bin/conda-build", line 4, in <module>
    import conda_build.cli.main_build
  File "/Users/arokem/anaconda3/lib/python3.5/site-packages/conda_build/cli/main_build.py", line 16, in <module>
    import conda_build.api as api
  File "/Users/arokem/anaconda3/lib/python3.5/site-packages/conda_build/api.py", line 22, in <module>
    from conda_build.config import Config, get_or_merge_config, DEFAULT_PREFIX_LENGTH as _prefix_length
  File "/Users/arokem/anaconda3/lib/python3.5/site-packages/conda_build/config.py", line 14, in <module>
    from .conda_interface import string_types, binstar_upload
  File "/Users/arokem/anaconda3/lib/python3.5/site-packages/conda_build/conda_interface.py", line 21, in <module>
    from conda.resolve import MatchSpec, NoPackagesFound, Resolve, Unsatisfiable, normalized_version  # NOQA
ImportError: cannot import name 'NoPackagesFound'
@skoudoro

This comment has been minimized.

Member

skoudoro commented Aug 9, 2017

With OS X, you have to use the clinicalgraphics channel

conda install -c clinicalgraphics vtk

@arokem

This comment has been minimized.

Member

arokem commented Aug 9, 2017

Huh. Interesting. That doesn't work with Python 3.5 (which is what I am using atm), but does seem like it would work with 3.6. Thanks for pointing that out. Even so: I don't think we want this level of detail in the README.

@coveralls

This comment has been minimized.

coveralls commented Aug 9, 2017

Coverage Status

Changes Unknown when pulling 6a0f8e0 on arokem:no-gpl into ** on nipy:master**.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Aug 9, 2017

Coverage Status

Changes Unknown when pulling 6a0f8e0 on arokem:no-gpl into ** on nipy:master**.

@grlee77

This comment has been minimized.

Contributor

grlee77 commented Aug 9, 2017

I can't get the more detailed instructions you provided to work for me

I have not seen that error, but it could be related to the version of conda or conda-build used. conda-forge currently uses conda 4.3.22 and conda-build 2.1.17. You can install those versions via:

conda install -c conda-forge conda conda-build

(These conda tools must be installed into the base anaconda environment)

@skoudoro

This comment has been minimized.

Member

skoudoro commented Aug 9, 2017

I agree, we do not need this level of detail in the README.

vtk installation seems to be quite limited with conda on OSX. (only python 3.6, not even python 2.7)

README.rst Outdated
pip install dipy
or using `conda` (only on Python 2):

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 25, 2017

Member

Hello. As far as I know there are no issues with Python 3. Please remove.

This comment has been minimized.

@arokem

arokem Aug 25, 2017

Member

Did something change since our conversation two weeks ago?

This comment has been minimized.

@grlee77

grlee77 Aug 25, 2017

Contributor

@arokem : The issue was OS X, not Python 3. The current availability of VTK 7.1.1 is Python 2.7, 3.5 and 3.6 for linux-64, win-32 and win-64. There are no conda-forge VTK packages for OS X (or for 32-bit linux).

I did recently upload VTK 7.1.1 packages built for OS X in my channel if you want to try them out. As of now these match the conda-forge recipe, but these have not undergone the automated CI testing process and I can't guarantee to always keep them in sync going forward, so I wouldn't mention them in the README.

@arokem

This comment has been minimized.

Member

arokem commented Aug 25, 2017

@coveralls

This comment has been minimized.

coveralls commented Aug 25, 2017

Coverage Status

Changes Unknown when pulling e009030 on arokem:no-gpl into ** on nipy:master**.

README.rst Outdated
MIT licenses, with the only exception of the SHORE module which
optionally uses the cvxopt library. Cvxopt is licensed
under the GPL license.
We welcome contributions from the community. Please read our `contributor guidlines <https://github.com/nipy/dipy/blob/master/CONTRIBUTING.md>`.

This comment has been minimized.

@skoudoro

skoudoro Sep 20, 2017

Member

guidlines to guidelines

This comment has been minimized.

@skoudoro

skoudoro Sep 20, 2017

Member

I think, it is ready to go after this correction. Any other suggestions ?

@arokem arokem force-pushed the arokem:no-gpl branch from 2287af7 to c381fcd Sep 20, 2017

@Garyfallidis Garyfallidis merged commit 5d926b5 into nipy:master Sep 26, 2017

3 checks passed

codecov/patch Coverage not affected when comparing d737a6a...c381fcd
Details
codecov/project 87.02% remains the same compared to d737a6a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Sep 26, 2017

Thanks! Free again :)

@arokem

This comment has been minimized.

Member

arokem commented Sep 26, 2017

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1315 from arokem/no-gpl
Remove GPL from our README.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment