Skip to content

experiment with better describing the file finding #343

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

Merged
merged 5 commits into from
Nov 26, 2019

Conversation

RonnyPfannschmidt
Copy link
Contributor

No description provided.

@RonnyPfannschmidt
Copy link
Contributor Author

https://twitter.com/ossronny/status/1144101215724331009 has some details/hint on how to proceed, will pick it up again later

@RonnyPfannschmidt RonnyPfannschmidt self-assigned this Jun 27, 2019
Copy link
Contributor

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor spelling/wording suggestions

RonnyPfannschmidt and others added 3 commits June 27, 2019 08:07
@zaneselvans
Copy link

I will try and summarize my comments here so they are easier to track.

The only current mention of the file inclusion functionality is a single line "It also handles file finders for the supported SCMs.", whereas essentially all external references to setuptools_scm and 99.9% of the documentation bills it as a convenient way to single-source the package version. This makes the file inclusion quite surprising, even to the point of seeming inexplicable, especially to a newer user.

This surprising behavior only compounds the complexity of the several overlapping mechanisms for including and excluding files from the distribution -- MANIFEST.in, the MANIFEST directly, include_package_data, the find_packages() function, as well as the way the inclusion of data files from outside the python package itself has gone from being an apparently acceptable practice, to something that is almost forbidden, as well as the different kinds of distributions (sdist, bdist, bdist_wheel?) that (apparently?) include different kinds of files? Like I still don't really understand why it would be reasonable to include everything from the repository in the sdist -- the repo I am trying to package is a very active shared workspace with a bunch of other non-code files that are related to the project, but clearly not appropriate for distribution with the library we are developing (including some binary files, a bunch of Jupyter notebooks, etc.)

At the same time, the value of ensuring that every single file distributed in the library being packaged is checked into source control, and that the version of the file which was included in the built distribution is in fact the version of that file which is associated with the tagged version in the repository is very clear.

The file inclusion functionality is a big thing that setuptools_scm does, and it is likely not the functionality that someone coming to the project is looking for, given the prominence of the version tracking in links to the project, so highlighting this behavior seems very important. I think it clearly warrants a top-level header section in the README, that tries to briefly explain, or at least provides links to fuller resources explaining:

  • What it means to provide a "file finder" What do they do? How is the file finding that setuptools_scm does different from what the other file finding options do?
  • Why might one want to include every single file in source control in the sdist (and also, why one might not want to).
  • How and why are different sets of files included in different types of distributions (sdist, bdist, wheel, etc.)?
  • How to override the default behavior if that's what the user wants to do (e.g. if they already have a working collection of rules in MANIFEST.in and other mechanisms that they do not want to disrupt, and are only seeking to link the package version directly to source control).
  • What are the implications of going ahead with distributing every file in source control in the sdist, since that's what setuptools_scm does by default. What if there's ~100MB worth of files, some of them binary, checked into the repository? Just. You know. Hypothetically speaking. Would that be a reasonable thing to upload to PyPI?
  • If it's not appropriate to include every single source controlled file in the repo in the distribution, how might one reasonably choose which files to exclude, and which files to include?

Part of what I'm getting at here is, there are lots of different and totally reasonable repository usage patterns. We initially organized our repository based on the recommendations of @gvwilson and others, as generally laid out in:

But using the repository as a shared, version controlled research workspace, and using the repository as a feedstock for python package distribution are two different modes (and I'm sure there are others as well!), and having the documentation take into account the possibility that someone is coming to the task from a different pattern of use would help ease the transition.

Anway, thanks for your work on this! It is obviously a good tool. I suspect the only problem is that the functionality has outrun the documentation!

@RonnyPfannschmidt
Copy link
Contributor Author

@zaneselvans thanks for this extensive write-up, i just want to ping in to say this hasn't been forgotten, i just didn't have as much of personal time to spend on setuptools_scm in the last month as id like

Co-Authored-By: Sviatoslav Sydorenko <[email protected]>
@webknjaz
Copy link
Member

@RonnyPfannschmidt I think you should merge this as is and seek the perfection in the follow-up iterations. It already improves the experience in the current state :)

@RonnyPfannschmidt RonnyPfannschmidt merged commit bf07307 into master Nov 26, 2019
@RonnyPfannschmidt RonnyPfannschmidt deleted the RonnyPfannschmidt-doc-file-finder branch January 8, 2020 19:57
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.

4 participants