poetry icon indicating copy to clipboard operation
poetry copied to clipboard

Inconsistent handling of include / exclude for sdist and wheel

Open RobbeSneyders opened this issue 1 year ago • 8 comments

  • Poetry version: 1.7.1
  • Python version: 3.10.12
  • OS version and name: Ubuntu 22.04
  • pyproject.toml: https://github.com/ml6team/fondant/blob/4a186967e860719e0385f0e1f5010814d8792e49/pyproject.toml
  • [x] I am on the latest stable Poetry version, installed using a recommended method.
  • [x] I have searched the issues of this repo and believe that this is not a duplicate.
  • [x] I have consulted the FAQ and blog for any relevant entries or release notes.
  • [x] If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

When building our package, we want to include only yaml files and ignore all other files from a certain subdirectory. The relevant part of our file tree looks like this:

├── src
|   ├── fondant
|   |   ├── components
|   |   |   ├── component_1
|   |   |   |  ├── fondant_component.yaml
|   |   |   |  └── bunch of other files and dirs
|   |   |   └── component_2
|   |   |   |  ├── fondant_component.yaml
|   |   |   |  └── bunch of other files and dirs
|   |   ├── some other directories
|   |   └── some python files
└── pyproject.toml

See our full file tree in our repo, the relevant directories and files are called the same.

We only want to include the fondant_component.yaml file from each component in src/fondant/components and ignore all other files.

I tried to do this with the following include / exclude in our pyproject.toml:

include = ["src/fondant/components/**/*.yaml"]
exclude = ["src/fondant/components"]

So excluding the full directory, and including the specific files we do want to package.

However this leads to inconsistent results in the sdist and wheel outputs:

sdist

src
├── fondant
    ├── components
    |   ├── component_1
    |   |  ├── fondant_component.yaml
    |   └── component_2
    |      ├── fondant_component.yaml
    ├── some other directories
    └── some python files

This is the result we want to achieve.

wheel

├── fondant
|   ├── some other directories
|   └── some python files
└──src
   └── fondant
       └── components
           ├── component_1
           |  ├── fondant_component.yaml
           └── component_2
              └── fondant_component.yaml

As a reproducible example, you can run poetry build from the root of our repo.

I've tried different things to achieve this result:

  • Starting from the package as root as recommended in https://github.com/python-poetry/poetry/issues/1336
  • Using an exclude pattern for yaml in the exclude glob:
    include = []
    exclude = ["src/fondant/components/**/*[!.yaml]"]
    

but none of them worked.

The difference in behavior of include / exclude between sdist and wheel seems like a bug to me, but we would also be interested in a workaround to achieve what we are looking for.

RobbeSneyders avatar Feb 20 '24 11:02 RobbeSneyders

this difference is per the docs, or anyway it will be in the upcoming release - https://github.com/python-poetry/poetry/pull/8852

please close

dimbleby avatar Feb 20 '24 11:02 dimbleby

I'm not convinced that that is the same issue, or even that the new documentation is correct. In our case include does seem to work for both sdist and wheel without specifying the format explicitly.

If this was the issue, then I would expect the following include / exclude to fix it:

include = [{ path = "src/fondant/components/**/*.yaml", format = ["sdist", "wheel"] } ]
exclude = ["src/fondant/components"]

But the result is unchanged.

RobbeSneyders avatar Feb 20 '24 12:02 RobbeSneyders

This seems closer related to #7153.

include does not seem to work correctly for wheels when using a project layout with a src directory. Because it does include the files, but it includes them under the wrong path.

RobbeSneyders avatar Feb 20 '24 12:02 RobbeSneyders

#7153 shows no bug and should be closed

when you say that you want to include src/whatever, poetry takes you at your word and includes it. I don't know a way to say that you want to include-and-move src/whatever

dimbleby avatar Feb 20 '24 14:02 dimbleby

I do think https://github.com/python-poetry/poetry/issues/7153 shows a bug, as do other users based on the comments.

Poetry's behavior is currently inconsistent:

  • for include between sdist and wheel See the example I added in my original message. The include path is added as a different path in sdist and wheel. This leads to a working installation with sdist and a broken installation with wheel.
  • for wheel between include and exclude See again my original example. I define both my include and exclude paths with src as the root. For exclude, the fact that the fondant package is located in the src directory is correctly taken into account, and fondant/components is excluded. For include, this is not taken into account, and src/fondant/components is included instead.

RobbeSneyders avatar Feb 20 '24 14:02 RobbeSneyders

if you want to argue about #7153, please do it in that issue

as we started: that include applies differently to sdist and wheel is known and documented

I don't know whether you've found a new wrinkle where saying "exclude" activates the "include" or something like that, maybe. Feel free to experiment and tease out what is going on, and even submit a pull request: the code is round about here

dimbleby avatar Feb 20 '24 14:02 dimbleby

if you want to argue about https://github.com/python-poetry/poetry/issues/7153, please do it in that issue

You're right. I did not find that issue before opening this one. I will move the discussion there, but would appreciate it if you could actually look into the arguments provided by myself and other users, or at least providing your own clear argumentation instead of being dismissive.

as we started: that include applies differently to sdist and wheel is known and documented

The documented difference is not correct though. It says that include is not applied by default to wheel, while it is applied. Just incorrectly.

RobbeSneyders avatar Feb 20 '24 14:02 RobbeSneyders

In the other one, you asked me to confirm that this one is a bug. (Aside: can we please discuss issue X in issue X, and issue Y in issue Y?)

I don't think there's a definitive answer, sorry. I don't understand exactly what the code is doing, why it is doing that, or whether it was intended to do that. If you're able to propose a definite improvement, I assume maintainers would be open to accepting it.

dimbleby avatar Feb 20 '24 14:02 dimbleby