Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.4k
Add data_files support in setup.cfg#1520
Conversation
setuptools/tests/test_config.py Outdated
| ) | ||
| with get_dist(tmpdir) as dist: | ||
| assert dist.data_files == [ |
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.
I believe this is failing becausedist.data_files gets parsed first to a dict (with arbitrary order on Py < 3.6), then turned into this key/value list thing.
I would look at what the other tests do, but I think a quick fix would be this:
expected= {'cfg': ['a/b.conf','c/d.conf'],'data': ['e/f.dat','g/h.dat']}withget_dist(tmpdir)asdist:# Order independent comparisonassertdict(dist.data_files)==expected
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.
Thanks a lot for your suggestion! I completely forgot about that. I'll commit the fix into this my branch.
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.
I committed the fix based on your suggestion but data type of expected result is kept.
In the test case, dist.data_files needs to be sorted because thecurrent implementation loads the configuration files as a dictionarywith arbitrary order on Python < 3.6.
This adds the `[options.data_files]` section to the existing setup.cfgexample.
pganssle left a comment
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.
I've rebased this branch against master and squashed together the fixup commits, plus touched up some of the commit messages and the changelog entry, so everything looks good to me.
Thanks@ssato for your PR!
benoit-pierre commentedOct 24, 2018
The documentation should be updated. |
pganssle commentedOct 24, 2018
@benoit-pierre Good catch, I guess we need to add it tothis table. Anywhere else? |
benoit-pierre commentedOct 24, 2018
Yes, and I would add a new column to the table mention the minimum supported version. |
pganssle commentedOct 24, 2018
@ssato If you want to update the documentation, please do a force-pull from your branch, since I've rewritten the PR's history since your last commit, and that will cause conflicts. |
ssato commentedOct 24, 2018
@pganssle Thanks a lot for the cleanups! It looks much better ;-) |
jaraco left a comment
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.
I think there's still an outstanding request to update the documentation.@ssato would you do that please?
pganssle commentedOct 25, 2018
@jaraco@benoit-pierre Normally I'd block on getting an update, but given that we're doing a sprint this weekend, I'm tempted to just merge this as-is and leave the documentation update as an easy If no one takes it at the sprints this weekend, I can do it myself on Monday, but frankly it's nice to have some very simple fixes that you can use as a demo PR for new contributors. |
jaraco commentedOct 25, 2018
Works for me.@pganssle Would you file that ticket? |
pganssle commentedOct 25, 2018
Filed#1522. |
dirk-thomas commentedJan 4, 2019
Since |
Uh oh!
There was an error while loading.Please reload this page.
Summary of changes
These commits add data_files support in setup.cfg, discussed in the issue#1189, such like the following.
I think it mayclose#1189.
Pull Request Checklist