Add support for data_files #890 #901
Conversation
|
I'll fix the test failures first thing tonight. The source of the problem is that maps retain insertion order as of python 3.6, but not in 2.7 & 3.5. |
* data_files were sorted inconsistently between pythons <3.6 and >=3.6 * generated setup.py included platform specific paths to data_files
* textwrap.indent isn't available for python 2.7
|
@sdispater are there any additional tests or things you'd like addressed in this PR? |
|
@delphyne, for what it's worth, I submitted a similar PR a few months ago, #539. I haven't compared them in detail, but it may be worth incorporating some things from mine (e.g., mine includes changes to poetry-schema.json). It looks like mine has some conflicts now, so yours is probably a better base since it's closer to the current code. |
|
@mtkennerly that's unfortunate! Also a little unfortunate that your PR has been hanging out for so long without any comment. I tried to search for open issues and PRs for this functionality, but I didn't find your PR, so sorry for the duplicated effort. What is the purpose of the poetry-schema file? all the tests pass without the changes, and it's not mentioned in the contributing docs. |
|
@sdispater do you have help with this project? Seems like you might be overwhelmed with 76 outstanding pull requests and 422 open issues as of this comment. Do you need or want help? Who do you trust to perform code reviews? |
There was a problem hiding this comment.
Thanks so much for making this contribution!
Just a few questions:
- Do we also need to update
poetry/json/schemas/poetry-schema.jsonas was done in Add support for data files #539 ? - Can you compare to "Search for package" during poetry init does not work behind a corporate proxy #531 and note any important differences?
After that we can hope to steal some time from sdispater to review this. I'm not yet qualified to be approving features.
|
|
||
| ## data_files | ||
|
|
||
| A list of files to be installed using the [data_files](https://docs.python.org/2/distutils/setupscript.html#installing-additional-files) |
There was a problem hiding this comment.
Lets link to the Python 3 version?
| @@ -40,6 +40,7 @@ def __init__(self, poetry, env, io): | |||
| self._path.as_posix(), | |||
| packages=self._package.packages, | |||
| includes=self._package.include, | |||
| data_files=self._package.data_files, | |||
There was a problem hiding this comment.
How does this compare to
https://github.com/sdispater/poetry/pull/539/files#diff-adffe92a131c3e1bf7e56fb29b1bc265R134
| @@ -14,6 +14,7 @@ def __init__(self, name, version, pretty_version=None): | |||
| self.packages = [] | |||
| self.include = [] | |||
| self.exclude = [] | |||
| self.data_files = {} | |||
There was a problem hiding this comment.
why a {} vs [] as compared to https://github.com/sdispater/poetry/pull/539/files#diff-79da47ec0c1b34186a766622ab6e78ffR17
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This should now be covered by https://python-poetry.org/docs/pyproject/#include-and-exclude. If this is not the case, please feel free to rebase and open a new PR. |
|
@delphyne / @mtkennerly are you able to update/recreate this PR? it's still relevant, as per the issue discussion. |
|
I could work on it, but I'd appreciate some confirmation first that the changes would be considered for merge. I haven't seen a follow-up from @abn in #890 regarding the latest comments. @abn or @sdispater, could you comment as to whether you're open to a PR that adds support for |
|
Yes, that's very reasonable! Well consider this my endorsement anyway, for
whatever that's worth to anyone!
…On Mon, 20 Jul 2020, 20:26 Matthew Kennerly, ***@***.***> wrote:
I could work on it, but I'd appreciate some confirmation first that the
changes would be considered for merge. I haven't seen a follow-up from
@abn <https://github.com/abn> in #890
<#890> regarding the latest
comments. @abn <https://github.com/abn> or @sdispater
<https://github.com/sdispater>, could you comment as to whether you're
open to a PR that adds support for data_files based on the use cases
described in the most recent comments in #890
<#890>?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#901 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7SILUXK3TFD2A57XHBJQ3R4SK6LANCNFSM4GYPWP2A>
.
|
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Adds support for data_files to poetry.
Addresses enhancement request #890
Note: Black made a number of formatting changes in files that I touched, but in sections I wasn't editing.