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

Install-time requirements for pip and conda #577

Merged
merged 6 commits into from
Mar 3, 2015

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Feb 24, 2015

This PR adds some checking for dependencies at install time. Specifically,

  • GEOS header/libraries using the geos-config program.
  • Proj4 using pkg-config or conda.
  • Add some text files specifying requirements. These should be compatible with both pip and conda, and are also parsed to provide the install_requires and extra_requires so that pip install or python setup.py install will automatically install dependencies.

I cannot test on OS X or Windows; consequently, I have not made failure to find GEOS or Proj4 fatal. If someone can confirm the tests are sufficient for those platforms, then I can do so.

The split between install and extras is somewhat arbitrary based on the INSTALL file. Suggestions are welcome there.

exit(1)

proj_includes = proj_includes.split()
proj_libraries = []
Copy link
Member

Choose a reason for hiding this comment

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

These defaults should take us back to what we had before. i.e. proj_libraries = ['proj']

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, quite right.

@pelson
Copy link
Member

pelson commented Feb 24, 2015

In principle this is great 👍
Whether it is a good idea to do this right before a release is a reasonable question - I'll try to spin out the Windows laptop and see if everything works as expected later on.

@QuLogic
Copy link
Member Author

QuLogic commented Feb 24, 2015

Whether it is a good idea to do this right before a release is a reasonable question

I've still got a week, right... ;) But anyway, I figure we should try and get all the install-related issues squared away with the setuptools changes.

@pelson
Copy link
Member

pelson commented Feb 24, 2015

I've still got a week, right... ;) But anyway, I figure we should try and get all the install-related issues squared away with the setuptools changes.

It didn't take much. Yes, I agree - we're issuing a release candidate (today I hope) for this very reason.

@QuLogic
Copy link
Member Author

QuLogic commented Feb 24, 2015

OK, I think that should fix up your comments.

@ajdawson
Copy link
Member

Tests are failing due to an SSL error when accessing binstar. I'm actually seeing the same error on my local machine when I try to upload binstar packages. Anyone have any idea if we can fix this?

@ocefpaf
Copy link
Member

ocefpaf commented Feb 24, 2015

@pelson
Copy link
Member

pelson commented Feb 24, 2015

Thanks @ocefpaf. @QuLogic - would you mind putting the config option into your PR?

@ocefpaf
Copy link
Member

ocefpaf commented Feb 24, 2015

In case you guys are not on the conda mailing list. The latest request update should fix this.

To keep compatibility with conda, the requirements files for optional
packages do not specify install of the defaults.

Fixes SciTools#576.
Also, ensure error messages go to stderr.
On Fedora 20, proj 4.8.0 _is_ available, but does not contain the
proj.pc file for pkg-config.
@QuLogic
Copy link
Member Author

QuLogic commented Feb 24, 2015

I've rebased to remove the extra edits, and put in some fixes for Python 3. The new build should pull in the updated requests package.

return proj_version


conda = os.getenv('CONDA_DEFAULT_ENV')
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely comfortable with having conda special cases in the setup.py. I'm trying to get my head around what not having proj in pkg-config actually means.

Instead of checking for conda when we can't find the pkg-config information, is it worth seeing if we can find the proj executable, and working from there? After all, proj version can be determined with:

$> proj
Rel. 4.8.0, 6 March 2012

Copy link
Member Author

Choose a reason for hiding this comment

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

The conda check is not a fallback; it's an override. The trouble is that conda provides pkg-config files, but does not provide the script, nor does it override the search path.

What this means is, I will find the system install via pkg-config in place of conda's first, and never check proj. That will possibly cause compiling/linking against system proj over conda's (possibly this depends on the full linker path after everything's put together, of course.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably the more correct fix is to have conda either provide pkg-config or set PKG_CONFIG_PATH.

Copy link
Member

Choose a reason for hiding this comment

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

But conda does set PATH, so we could check to see if proj == the proj found with pkg-config. I don't know which is preferable though - the one on the PATH or the one defined by pkg-config...

Copy link
Member Author

Choose a reason for hiding this comment

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

pkg-config will only give you header and library path, and these are not necessarily easily linked to the binary's path. One might assume bin/include/lib or lib32 or lib64, but really autoconf (used by proj's install) allows you to override any of these.

pelson added a commit that referenced this pull request Mar 3, 2015
Install-time requirements for pip and conda
@pelson pelson merged commit c9be5e8 into SciTools:v0.12.x Mar 3, 2015
@pelson
Copy link
Member

pelson commented Mar 3, 2015

Thanks @QuLogic - this is a big change, but I think you've nailed it! I'm going to kick off a build on Windows and keep my fingers crossed...

@QuLogic QuLogic added this to the v0.12.0 milestone Mar 9, 2015
@QuLogic QuLogic deleted the requirements branch March 9, 2015 21:55
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.

None yet

4 participants