Skip to content

BLD: Remove options.package_data from setup.cfg #46270

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

Closed
NickCrews opened this issue Mar 8, 2022 · 0 comments · Fixed by #46271
Closed

BLD: Remove options.package_data from setup.cfg #46270

NickCrews opened this issue Mar 8, 2022 · 0 comments · Fixed by #46271
Labels
Build Library building on various platforms
Milestone

Comments

@NickCrews
Copy link
Contributor

NickCrews commented Mar 8, 2022

Hello! Not sure if this is actually what we want, but I think this can simplify things.

Currently, we include data files in builds in 3 ways:

  1. Listing them in MANIFEST.in
  2. Listing them under options.package_data in setup.cfg
  3. Setting include_package_data = True in setup.cfg

I believe that it is possible (and therefore desirable, as it is more DRY and less spread out) to migrate everything in number 2 to number 1, and still have the same behavior.

This is per both https://setuptools.pypa.io/en/latest/userguide/datafiles.html and from experiments in https://github.com/abravalheri/experiment-setuptools-package-data, where it is shown that if you have include_package_data = True, then MANIFEST.in is sufficient, you don't need options.package_data, both for sdists and wheels.

It looks like include_package_data = True quietly snuck during #38852. Before this, I think the package_data was needed, but now it is not.

A small local test I think showed this to be true, but I might have messed it up so not sure. Probably artifacts from CI would tell the story.

@fangchenli I think you might be the one to look at this? Thank you! See the attached PR

NickCrews added a commit to NickCrews/pandas that referenced this issue Mar 8, 2022
These two glob patterns are already covered because:
- They are listed in MANIFEST.in
AND
- include_package_data is set to True

See pandas-dev#46270
@fangchenli fangchenli added the Build Library building on various platforms label Mar 8, 2022
NickCrews added a commit to NickCrews/pandas that referenced this issue Apr 9, 2022
These two glob patterns are already covered because:
- They are listed in MANIFEST.in
AND
- include_package_data is set to True

See pandas-dev#46270
@jreback jreback added this to the 1.5 milestone Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants