-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
html5lib: bump version #4
Conversation
nehaljwani
commented
Jul 27, 2017
•
edited
Loading
edited
- Add another import in test section
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
recipe/meta.yaml
Outdated
@@ -13,12 +13,12 @@ source: | |||
|
|||
build: | |||
number: 0 | |||
script: pip install --no-deps . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are moving towards using pip
. So this is "future" correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ocefpaf, can you detail why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few feedstocks that only do the right thing about its entrypoints
when pip
installed. See the nbgrader
issue as an example. The same happens with other jupyter
related feedstocks and that is why @minrk added pip
here. (Although this particular feedstock does not suffer from that problem.)
@msarahan and I can had a brief chat about this at SciPy but nothing is "formalized" yet. We basically need to know what means for conda-build
the change from python setup.py install
to pip install --no-deps .
(Actually I found out that we need to use python -m pip install --no-deps .
in some cases, not sure why :-/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this issue for discussion of pip vs setup.py.
The gist is that pip is really the only official way to install Python packages these days, and anything else is deviating from the norm and should be minimized. Every time a conda-installed package differs in behavior from its pip-installed counterpart is generally considered a bug in the conda package, so keeping the differences in process minimal is desirable from a user and maintenance perspective.
There are two differences between pip and setup.py at this point (that I know of):
- if a package doesn't use setuptools you can't use the extra arguments, and if it adopts setuptools, you must use them to avoid installing an egg. This means that conda recipe maintainers must know whether the package uses setuptools in setup.py or not. pip will install correctly in both cases.
- For packages with entrypoints, setuptools writes entrypoints that use
pkg_resources
, making setuptools itself a runtime dependency, plus dramatically slowing down invocation. pip has fixed this, and creates entrypoints that do not add any dependencies and launch more quickly.
In every known (to me) case where pip and setuptools differ, pip behavior is preferable. Further, any future further deviations between pip and setuptools should be picked up by conda packages, so calling pip makes more sense to me than directly invoking setup.py and then trying to keep track of every change in pip and duplicating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@minrk I am compiling this info into a CFEP now so you don't have to repeat it 😄
@mingwandroid and @msarahan we can move the discussion to the CFEP. I'll ping you all there as soon as I have the PR.
@nehaljwani can you revert just this change so we can merge this? (If you are busy I can do that for you.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ocefpaf I have updated the PR
8691ad6
to
91d23d5
Compare
- Add another import in test section
Thanks @nehaljwani. |