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

html5lib: bump version #4

Merged
merged 1 commit into from
Jul 28, 2017
Merged

Conversation

nehaljwani
Copy link
Member

@nehaljwani nehaljwani commented Jul 27, 2017

  • Add another import in test section

@conda-forge-linter
Copy link

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) and found it was in an excellent condition.

recipe/meta.yaml Outdated
@@ -13,12 +13,12 @@ source:

build:
number: 0
script: pip install --no-deps .
Copy link
Member

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.

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?

Copy link
Member

@ocefpaf ocefpaf Jul 27, 2017

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 :-/)

Copy link
Member

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):

  1. 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.
  2. 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.

Copy link
Member

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.)

Copy link
Member Author

@nehaljwani nehaljwani Jul 28, 2017

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

@nehaljwani nehaljwani force-pushed the bump-version branch 2 times, most recently from 8691ad6 to 91d23d5 Compare July 28, 2017 11:38
- Add another import in test section
@ocefpaf ocefpaf merged commit e5558fa into conda-forge:master Jul 28, 2017
@ocefpaf
Copy link
Member

ocefpaf commented Jul 28, 2017

Thanks @nehaljwani.

@nehaljwani nehaljwani deleted the bump-version branch July 30, 2017 16:23
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.

5 participants