Skip to content
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

Update instructions since pip has removed the --process-dependency-links option #1201

Merged
merged 7 commits into from
Nov 8, 2016

Conversation

duncan-brown
Copy link
Contributor

Add the --process-dependency-links option to pip to bring in the dependencies that PyCBC needs.

@samantha-usman
Copy link

I don't know what dependency links are. What should this option take in? What would the error message be without it?

@duncan-brown
Copy link
Contributor Author

duncan-brown commented Nov 8, 2016

@samantha-usman ok, this is more complicated that I thought. --process-dependency-links was an option to pip that would allow a package from github to bring in another package. This is used to install mpld3 as part of the PyCBC install, see:

https://github.com/ligo-cbc/pycbc/blob/master/setup.py#L73

However, it appears that the pip maintainers have removed this feature in pip 9.0 (see pypa/pip#1519 and pypa/pip#3939).

The solution to this is to add a line to the instructions telling the user to install mpld3, like we do for the segment database tools.

I will update with that patch in a few minutes.

@duncan-brown
Copy link
Contributor Author

@samantha-usman I have updated the instructions to tell you do a an explicit install of mpld3:

pip install https://github.com/ligo-cbc/mpld3/tarball/master#egg=mpld3-0.3git

as part of the installation of required packages. I removed the deprecated --process-dependency-links argument.

@ahnitz I also removed it from the setup.py file

@samantha-usman please try this and see if it works. Please comment here if it works or fails, or if you are having issues with the instructions.

@duncan-brown duncan-brown changed the title Tell pip to bring in all the dependencies Update instructions since pip has removed the --process-dependency-links option Nov 8, 2016
ahnitz
ahnitz previously requested changes Nov 8, 2016
@@ -70,7 +70,6 @@
'corner>=2.0.1',
#'scikit-learn>=0.17.0', # travis does not like scikit-learn
]
Copy link
Member

Choose a reason for hiding this comment

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

do not remove from the setup.py. It is required afterall. Even if you are installing from outside pycbc, we should not remove the dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that directive has been deprecated in both setuptools and pip. We shouldn't have it there if it's a broken noop. If you want it in setup.py, the right thing to do is release a pycbc-mpld3 in PyPi and then add it to the regular requirements.

@duncan-brown duncan-brown dismissed ahnitz’s stale review November 8, 2016 13:51

mpld3>=0.3git is still listed as a requirement so the user must have manually installed it

@ahnitz
Copy link
Member

ahnitz commented Nov 8, 2016

@duncan-brown I think this is the better solution.

#1204

I

@duncan-brown
Copy link
Contributor Author

duncan-brown commented Nov 8, 2016

I incorporated #1204 here so that it also gets the other fixes to the documentation.

@ahnitz ahnitz merged commit 32b9c0b into gwastro:master Nov 8, 2016
@duncan-brown duncan-brown deleted the pip_bug branch November 14, 2016 04:15
hagabbar pushed a commit to hagabbar/pycbc_copy that referenced this pull request May 24, 2017
…nks option (gwastro#1201)

* Tell pip to bring in all the dependencies

* Fixed typo that hid instructions to update pip

* Point to new DASWG repository page

* removed link as process dependency links has been removed from pip

* fix instructions to remove process-dependency-links

* removed deprecated dependency_links

* get mpld3 from pypi
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.

4 participants