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

Don't import dependencies in setup.py #285

Closed
asmeurer opened this issue Sep 8, 2014 · 9 comments
Closed

Don't import dependencies in setup.py #285

asmeurer opened this issue Sep 8, 2014 · 9 comments

Comments

@asmeurer
Copy link

asmeurer commented Sep 8, 2014

Don't import the dependencies (numpy, maptlotlib, scipy, pandas) in setup.py. Rather, specify them in the install_requires in setup(). This makes pip installing seaborn impossible unless you already have those packages installed.

@mwaskom
Copy link
Owner

mwaskom commented Sep 8, 2014

This ground has been extensively covered in #169 and #277

@mwaskom mwaskom closed this as completed Sep 8, 2014
@asmeurer
Copy link
Author

asmeurer commented Sep 8, 2014

FYI this also causes conda skeleton pypi seaborn to generate an incorrect recipe (although it should fail to build a broken package until you fix it).

My opinion is that even if you don't like pip's behavior, you should do the recommended thing, because people using pip will expect pip to behavior as it behaves. If people don't like how pip works, tell them to use conda (which also updates dependencies by default BTW, but there are ways to make it not do so), but for people who want to use pip and like how it works, this is broken.

@mwaskom
Copy link
Owner

mwaskom commented Sep 8, 2014

I specifically refer you to #169 (comment)

@mwaskom
Copy link
Owner

mwaskom commented Sep 8, 2014

FWIW, I find conda's default dependency upgrade behavior to be very perplexing.

@asmeurer
Copy link
Author

asmeurer commented Sep 8, 2014

Just because you disagree with pip's behavior doesn't mean you should intentionally break your package.

From my experience with conda, most people don't care about automatic dependency upgrading, and for many (most?) it is a good thing because they would never update things otherwise, even important security updates like minor Python versions or openssl.

By the way, if you don't like the behavior in conda, there is no way yet to disable it entirely (the ratio of the work it would require to the number of people requesting it and the other things on my todo list), but you can pin specific packages, as described at http://continuum.io/blog/advanced-conda-part-1 (the "pinning packages" section).

@mwaskom
Copy link
Owner

mwaskom commented Sep 8, 2014

Whether a behavior is "broken" is a function of someone's expectations, and people have different expectations. For example:

[seaborn]{master}$ conda update pandas
Fetching package metadata: ..
Solving package specifications: .
Package plan for installation in environment /Users/mwaskom/anaconda:

The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    conda-3.6.3                |           py27_0         151 KB
    requests-2.4.0             |           py27_0         577 KB
    ------------------------------------------------------------
                                           Total:         728 KB

The first few times I encountered this situation, I thought "huh, I guess conda still has some bugs to work out." You clearly think there's a good argument for this behavior, and you're probably right! You've certainly thought more about this issue than me.

But my point is that there is a set of possible things that could have happened, and the thing that did happen is not the one I expected. I'm sure you would have been annoyed if I opened a conda issue just saying "this is broken!"

@asmeurer
Copy link
Author

asmeurer commented Sep 8, 2014

Conda updating itself is a separate issue from updating the dependencies. We manually inject conda into any install in the root conda environment so that it always stays up-to-date. This was a conscious decision on our part because otherwise most people would not update conda (and we only have the resources to support the latest version).

I'm sure you would have been annoyed if I opened a conda issue just saying "this is broken!"

How is pip install seaborn not working anything but broken? This isn't an issue of different behavior, this is an issue of a feature being intentionally disabled. For people who like pip and the way it works this is broken. For people who don't like it, they won't use pip anyway (or they will find a way to workaround the --upgrade behavior you describe).

This is like saying, "I don't like how os.path works. It's fundamentally broken. So whenever I import my library, I'm going to run import os.path; del os.path".

Believe me, I am not a user of pip, but seeing things like this really makes me doubt my own slide in my conda talk https://speakerdeck.com/asmeurer/conda-a-cross-platform-binary-package-manager-for-any-distribution?slide=14

@mwaskom
Copy link
Owner

mwaskom commented Sep 8, 2014

How is pip install seaborn not working anything but broken?

But you're missing that point that if I put numpy, matplotlib, etc. in install_requires, then pip install seaborn would also frequently not work. So again, imo all of the possible approaches are "broken". I've chosen for the time being to just follow the lead of packages like scipy and statsmodels, who have more time and resources to think about these issues. If you can convince them to adopt a different approach, I will too.

I also think that it's the best approach for seaborn because the vast majority of potential users will have the four core dependencies installed already and thus pip install seaborn will work perfectly well.

This isn't an issue of different behavior, this is an issue of a feature being intentionally disabled.

Yes, but in this case it's a "feature" that acts a lot like a bug.

As for the slide in your conda talk, if you read the issue thread I linked you to, this decision was made because someone could not install seaborn under the previous approach, which used install_requires. (And not just "someone", but a core statsmodels dev who knows a lot about dealing with Python installation issues). At the end of the day, getting an immediate import error and then being able to choose how to proceed yourself seems less annoying that downloding all of matplotlib only to get a perplexing compiler error.

@asmeurer
Copy link
Author

asmeurer commented Sep 8, 2014

By the way, all these dependencies except for numpy ship wheels on PyPI, so pip installing them should work much better now.

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

No branches or pull requests

2 participants