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

Simplify declaration of install_requires. #9536

Merged
merged 1 commit into from Oct 28, 2017

Conversation

Projects
None yet
5 participants
@anntzer
Copy link
Contributor

commented Oct 23, 2017

The previous way of declaring install_requires is way too overengineered
now that we unconditionally rely on the standard dependency resolution
machinery.

Don't use environment markers on install_requires as who knows what
version of setuptools & pip does the end user have.

Drop the check on Tornado as we don't do anything with it.

(Done in preparation of declaring a dependency on contextlib2 on Py2 to fix #9521 (comment).)

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way
Simplify declaration of install_requires.
The previous way of declaring install_requires is way too overengineered
now that we unconditionally rely on the standard dependency resolution
machinery.

Don't use environment markers on install_requires as who knows what
version of setuptools & pip does the end user have.

Drop the check on Tornado as we don't do anything with it.

@anntzer anntzer added the Build label Oct 23, 2017

@jklymak

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2017

Not sure if it units as a review, but I used this in a fresh environment on my machine and it installed fine using pip install -e .

@dopplershift
Copy link
Contributor

left a comment

I'm 👍 on the changes, but I'm not knowledgable about the previous system to know if there was some reason we had all that machinery.

@anntzer

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2017

I'm pretty sure the machinery dates back to when dependency installation was a mess and we used to vendor the dependencies, see https://github.com/matplotlib/matplotlib/wiki/MEP11#current-behavior.

@jklymak
Copy link
Contributor

left a comment

This works my setup and passes all the tests, which I think all download and setup, so I think this works.

@Kojoley Kojoley added this to the v2.2 milestone Oct 28, 2017

@Kojoley Kojoley merged commit e344d21 into matplotlib:master Oct 28, 2017

8 checks passed

ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing e10c523...176d6f5
Details
codecov/project/library 61.42% (target 50%)
Details
codecov/project/tests 98.72% remains the same compared to e10c523
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@anntzer anntzer deleted the anntzer:setuptools-install_requires branch Oct 28, 2017

@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.