-
-
Notifications
You must be signed in to change notification settings - Fork 611
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
Build requirements from setup.py #418
Build requirements from setup.py #418
Conversation
Without setup.py | ||
---------------- | ||
|
||
If you don't use `setup.py` ([you should][1]), you can write the following line to a file: |
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.
you should
This is an opinion not everyone would agree with.
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.
@dirn I read the post you reference about an hour ago while looking for a suitable URL for the link in question. I don't think it says we should or should not use setup.py
, I think the point was more that you shouldn't do stupid stuff like install_requires=list(open('requirements.txt'))
. Did I miss something?
The gist is that
|
@dirn I re-read the post. Nowhere does it say that setup.py should not be used with applications. The closest statement I could find was this:
It's difficult to know the authors intent, but the statement does not preclude the use of |
The two sections immediately before the quote you referenced are Python Libraries and Python Applications. The former covers We have, however, digressed from my original point a bit -- probably because I mistakenly replied out of context -- but my objection was to the use of "(you should)." Whether or not Donald was saying that you should or shouldn't use Perhaps you can explain your use case for having both
the
[1] Donald later references a sample |
When I wrote that post I struggled (and I still struggle) with trying to accurately describe, in my mind, the line between when you should use
A case where an application may have a
I'm not sure if that all makes sense, but I don't think that every thing should have a |
Having abstract requirements in I'll admit that this point of contention has been a bug-bear of mine since about 2008. I've never liked how Django messes with Frankly, the "my project doesn't need a setup.py" drivel has to stop. Writing a basic setup.py is easy, I've not heard a good reason to avoid doing so. Some lines from the Zen of Python are relevant here:
Applications are not special enough that we should advise that they omit having a The canonical way to define dependencies in Python is via the I'm taking a stand on this issue now - the Python packaging ecosystem has come a long way in the last 10 years, it's time to re-evaluate the status quo. Stop referencing any material written on the matter published more than a few months ago - it's likely based on ancient hearsay. The article presented by @dirn as a counter to my comment in the README was written in July 2013 - it's ancient. |
I am also not certain if we need to mandate (or absolutely recommend) the use of setup.py. requirements.in are sometimes an additionnal set of requirements related to the environment, but not necessarily needed by the package build as described by setup.py. It's not one or the other. @dstufft describes it well: if you need to install a webserver, you will not put that into the setup.py, as setup.py should describe what is needed by the package (be it a library or an application). Put in other words, I see the install_requires param as describing all the imports that will be made directly by running/using the package. I see this PR more helpful for libraries which typically do not use the requirements.in. It would help in reproducibility in build systems, but then again, this has drawbacks: it would also mask some errors that users of the library will have when using it unpinned (using the pinned requirements.txt for builds will isolate the more general open version usage, which can make subtle variations in sub dependencies on each install). Id rather catch a sub dependency breaking this library than let it be discovered by the user. |
What do I gain from shipping around an executable file whose sole purpose is to serve as the basis for the contents of a static
This is the nature of this entire conversation. Is the rule "everything needs a
Again, this calls into question what "it" is here. If it's "everything," then I try to write no more code than is necessary.
I'd argue that the way to do things "since about 2008" may be even less relevant, seeing as "the Python packaging ecosystem has come a long way in the last 10 years." |
Here's an example of a situation where I think Ansible modules rely on many packages available on PyPI. When I ship around Ansible playbooks, I will include a I prefer the rule
to
The first one feels better to me because, at the end of the day, both scenarios actually require asking "do I need a Two possible outcomes are easier for me to keep track of than three. Now I understand that this patch would keep |
I agree, this seems like a valid reason not to use I think I draw the line at the point where you expect others to import your modules, or you import your own modules. This is the point where namespaces come into effect. Simple scripts such as this serve as a prime example of where a namespsace isn't required, and hence there is no need for I don't consider public vs private distribution of packages to be relevant here, not all packages are uploaded to PyPi. Having installable requirements such as Would changing those 2 words from "you should" to |
Clearing up my previous comment, I feel the difference between a python script and a python module is that one isn't expected to be imported, the other is. The whole point of |
At the end of the day, the decision is Vincent's to make. I just didn't agree with what I thought was an overly general recommendation, especially since |
I've pushed a new commit, with a significantly toned down version of the offending line in the README. Regarding not using Is it time for @nvie to review? I'm happy to squash the commits in this or a separate branch if that's what is preferred. |
I'll just chime in with generally disagreeing with recommending setup.py. If it's the blanket recommendation, for applications it somewhat suggests "build requirements from installable-thing X, but don't install installable-thing X". Plus, for With .in files, it's trivial to define disjoint requirement sets, which can sometimes be useful (for e.g. generating docs, linting with mypy which may require a different version of python altogether, different pip versions via tox, etc). They may not even be capable of executing your code, which doesn't blend well with using |
Oh. I do like defaulting to |
@Groxx Given you commented after the second commit was pushed, I'm a little confused if you're +1 or -1 given that the recommendation has been removed. Can you please clarify? It looks like you object to the PR as it currently stands... |
Changes to the code: I'm not familiar enough to have an opinion on the actual implementation. The "fall back to setup.py" as a whole sounds convenient though, I like it. Automatically does the right thing for many library projects, which is great. Changes to the documentation, which prioritize setup.py: yes, somewhat opposed. It's a relatively significant high-level recommendation change, and I'm not convinced it's for an improvement. pip-compile is mostly for baking requirements.txt, which is most-important for applications in a production environment, not libraries (whose requirements.in or .txt will never be seen by the vast majority of users). |
@Groxx How do you feel the documentation prioritises Specifically, are you suggesting I swap the order of the two sub-sections within the Example usage for |
I've only read parts of the conversation and skimmed over the rest (there are several words). Sorry if I'm missing it, but what's the actual use case for this PR? When would you need both an $ pip install some-package
$ pip install -r path/to/site-packages/some-package/requirements.txt # ??? |
What'is the status of this? I would like to be able to use the install_requires in my existing setup.py to generate the requirements.txt, instead of having to have a separate requirements.in for pip-tools that duplicate those. |
@anthrotype: It seems the conversation about this PR devolved into philosophical discussion on the merits of Some concern was raised about my proposed changes to the documentation describing the new behaviour, and I made changes in accordance with the concerns raised. However, the detractors against the doc changes (@Groxx, @dirn, @davidovich) have not responded positively to the updated documentation. They haven't responded firmly against either (unless @Groxx eventually makes a response to my question about further changes to the docs), so I'd have to assume the updated docs are probably OK. If not, the detractors better speak up - we can assume consent through silence given they've had over 2 months to respond. @rpkilby asked a fundamental question:
I've decided to leave the answer to this question to people such as yourself. The reality is that this PR isn't likely to be accepted unless @nvie agrees with the changes, and Vincent hasn't made comment yet. Perhaps you can describe why this PR would be useful for yourself, it would clarify that there is indeed a use case for the proposed changes. |
tl;dr - I do not understand what problem this PR solves. In glossing over the thread, I have not seen an actual use case being discussed. @tysonclugg - can you provide an example of why this is beneficial to you?
I'm confused by this stance. Why would you propose changes without giving a reason as to why the changes are beneficial? Providing this reason seems like the best thing that can be done to advance interest in the PR.
The debate isn't just philosophical - it's about the practical use case of this PR. As I stated in my comment, I'm confused as to what this PR is trying to accomplish. Having a $ pip install some-package
$ pip install -r path/to/site-packages/some-package/requirements.txt # ???
Not sure if you're directing this at me, but I plainly stated that I don't understand what problem this PR solves. How would I be able to describe how this PR is useful to me? @tysonclugg - again, can you explain why this PR is useful to yourself? |
I don't think @tysonclugg was suggesting to distribute the requirements.txt to users. |
@rpkilby in short:
This abstract/concrete dependency model is the use case that this PR would resolve, and is also discussed in the related issue #331. I would very much like to use @davidovich would you reconsider merging the PR if the functionality is only enabled explicitly through a |
I think this has waited enough, and I am in favor of merging. Let's put this in the 1.9 release (there are two or three bug fixes I would like to land before incorporating features. (in retrospect, I was never against this ;-)) I think we have done enough bikeshedding. Caveat: The only thing missing now is tests. We must strive to augment coverage as this project has shared maintainership. |
I have implemented a basic test for this feature as requested (#481). Please let me know if there are any other cases that you would like covered. @tysonclugg please let me know if you would prefer to do this yourself, I'm just excited to see your work merged; my branch is built directly on yours. |
Thanks to all. This work was merged in #418, closing. |
This PR reads abstract requirements from the
install_requires
section ofsetup.py
, keeping things nice and DRY.Previous behaviour is maintained - if
requirements.in
is present, it will be used by default. If not, we use the new behaviour and read fromsetup.py
.