Include type information by default

[Danie-1]

Summary of changes

When getting source files, include py.typed and *.pyi as well as py files.

Closes #3136

Pull Request Checklist

• Changes have tests
• News fragment added in newsfragments/.
  (See documentation for details)

[Danie-1]

I'm not sure where to add tests because the tests that test functionality most similar to this seem to lie in _distutils (test_sdist.py has a test_add_defaults). Should this feature be in distutils instead?

[abravalheri]

Thank you very much @Danie-1 for the contribution ❤️ 💚

The errors with the docs are probably caused by the breaking changes in sphinx 7.2. I updated the main branch to temporarily cap at sphinx 7.1.2 while we fix the problems in a separated PR. You can rebase if you want to get rid of the CI errors.

Include type information (py.typed, *.pyi) by default

I just would like to align expectations in relation to the PR title and changelog entry:

It seems to me that what the PR does is to add .pyi and py.typed files automatically to the sdist, which is a fine contribution on its own.

But these files may not necessarily end-up on the wheel (I think that will depend on other factors like include_package_data, if the files are part of a folder, of if they are single file scripts at the root of the project, etc ...). I use the term may because that was just a strong suspicion of mine once I sat down to review the PR, but we can only really confirm that via testing and experimental data.

(I was suggesting something very similar in #3136 (comment), however upon further thought, I think both approaches would have the same shortcomings).

If you are OK with this behaviour, I think we can go ahead, but probably we will have to change the changelog entry to make this more explicit, so the other users don't create further expectations.
We also would have to leave the original issue open, since we don't fully solve the problem the user was reporting.

I'm not sure where to add tests because the tests that test functionality most similar to this seem to lie in _distutils (test_sdist.py has a test_add_defaults).

I think these could be added to setuptools/tests/test_sdist.py.

Should this feature be in distutils instead?

I don't think so. I think that the pypa/distutils repo accepts bugfixes, but not new features.

---

Some of the edge cases that I am not sure will not automatically add .pyi and py.typed to the wheel, even if they are in the sdist, are the following (non exhaustive list):

• What happens if the user has include_package_data = False?
• What happens if the user has py_module = ["myfile"], with myfile.py and myfile.pyi at the root of the project?

[Danie-1]

Thank you very much for the detailed comments.

It seems to me that what the PR does is to add .pyi and py.typed files automatically to the sdist, which is a fine contribution on its own.

But these files may not necessarily end-up on the wheel (I think that will depend on other factors like include_package_data, if the files are part of a folder, of if they are single file scripts at the root of the project, etc ...). I use the term may because that was just a strong suspicion of mine once I sat down to review the PR, but we can only really confirm that via testing and experimental data.

My intention with this was to make the files be included on the wheel by default (but I'll be completely honest and admit I have very little knowledge about python packaging and I don't know what the sdist is so I'll try to look into that). So at least at the moment, I would prefer to try to keep working on this until we can confirm that files are included in the wheel. In my pull request I should have said that this was intended to be work in progress (I have now marked it as draft).

About the experimental testing to confirm the behaviour, do you have any advice about how to go about this (are there any nice sources of small example packages to test with)? The way I tested these changes was to install my modified setuptools code into a venv and run python setup.py bdist_wheel and then pip install the created wheel and see how mypy interacted with it, but I wasn't confident at all that this was a sensible way to test it (in particular I would have liked to be able to use pip but I didn't know how to make it use my modified setuptools). I'll try to look into writing some unit tests as well.

I think these could be added to setuptools/tests/test_sdist.py.

I'll try to add some tests there. Would setuptools/tests/test_build_meta.py also be a good idea? I saw this line while browsing the tests a bit and thought it might be relevant for what you mentioned.

setuptools/setuptools/tests/test_build_meta.py

Line 420 in 5f14368

assert wheel_contents == {

• What happens if the user has include_package_data = False?

I've tested this and the files were not included, so I will try to address that.

I should have the time to have a go at all these things as I'm a student with a lot of free time, but I will soon be going on a holiday until September, so I won't be working on it during that time but I intend to work some more after that.

[abravalheri]

Hi @Danie-1, thank you very much for the comments and all the effort.

My intention with this was to make the files be included on the wheel by default (but I'll be completely honest and admit I have very little knowledge about python packaging and I don't know what the sdist is so I'll try to look into that). So at least at the moment, I would prefer to try to keep working on this until we can confirm that files are included in the wheel. In my pull request I should have said that this was intended to be work in progress (I have now marked it as draft).

Yeah, setuptools is a bit complicated because it has to keep backword compatibility with a wide range of packages published in the PyPI for the last years (should I say decade?).

Please feel free to iterate over the draft as much as you want, you can ping me if you need any advice. Once you think it is good to go, I can do a review.

About the experimental testing to confirm the behaviour, do you have any advice about how to go about this (are there any nice sources of small example packages to test with)?

If you want to make sure that a specific behaviour is observed, I recommend adding a test that captures that behaviour, so you can implement the change and check if the test passes.
There are a few examples on how we have been writing tests that check the contents of both sdist and wheel. For example https://github.com/pypa/setuptools/blob/v68.1.2/setuptools/tests/test_sdist.py#L812 or https://github.com/pypa/setuptools/blob/v68.1.2/setuptools/tests/test_build_meta.py#L315 ¹.

If you want just to investigate the existing behaviour, you can also create a separated set of scripts. For example, this is an experiment I did 2 years ago: https://github.com/abravalheri/experiment-setuptools-package-data

(in particular I would have liked to be able to use pip but I didn't know how to make it use my modified setuptools)

This is one way of using your modified version of setuptools with both pip and build (I am supposing the working copy of setuptools is in the modified_setuptools directory and that you have a dummy test project in the test_project directory):

python3.9 -m venv .venv  # creating a virtual environment to work as a sandbox
.venv/bin/python -m pip install -U pip wheel build  # let's update pip, wheel and build
.venv/bin/python -m pip install -e ./modified_setuptools  # install the working copy of setuptools

# If we disable isolation, both `pip`/`build` will use the version of setuptools
# that is currently installed in the virtual environment
.venv/bin/python -m build ./dummy_project --no-isolation
.venv/bin/python -m pip install ./dummy_project --no-build-isolation

# You can use `unzip -Z1` to list the contents of the wheel file
# and `tar tf` to list the contents of the sdist
unzip -Z1 dummy_project/dist/*.whl
tar tf dummy_project/dist/*.tar.gz

I should have the time to have a go at all these things as I'm a student with a lot of free time, but I will soon be going on a holiday until September, so I won't be working on it during that time but I intend to work some more after that.

No problems @Danie-1, it is very kind of you to be working on this and offering your contributions to the project. Please feel free to do it in your time.

Footnotes

1. If you intend to add tests to test_build_meta.py, it would be better to create a separated function like https://github.com/pypa/setuptools/blob/v68.1.2/setuptools/tests/test_build_meta.py#L964 instead of directly in the TestBuildMetaBackend (this class is a bit complicated and there is an implicit cartesian product between the number of methods and the number of tests that need to run, which make test_build_meta.py a very heavy/slow test file). ↩

[Danie-1]

I've now added some tests and an implementation that passes the tests. I think it addresses the things that were mentioned previously:

• What happens if the user has include_package_data = False?

  This is now checked in the tests.

• What happens if the user has py_module = ["myfile"], with myfile.py and myfile.pyi at the root of the project?

  PEP 561 says:

  This PEP does not support distributing typing information as part of module-only distributions or single-file modules within namespace packages.

Thanks for linking your repo explaining how data files get added and showing the --no-build-isolation flag; after reading your comments, some PEPs and more of the setuptools source code I have a bit of a better idea of how it all works.

I spent a while considering adding something like this in build_py.py:

        if self.packages:
            self.build_packages()
            self.build_package_data()
            self.build_type_information()  # new

but I was slightly hesitant because this would mean that type information files would be no longer treated as data files. I wasn't sure if this could break some things, and also I wasn't sure if this would make it harder to exclude type information files (if you really wanted).
However I think the main advantage of doing something like this is that I think it would be easier to add warnings about type information files that don't comply with PEP 561.

So the main thing I'd like to ask @abravalheri is whether you think it's worth doing the method I outlined above, or do you think the current implementation is sufficient?

[abravalheri]

Thank you very much @Danie-1, sorry for the delay in reviewing this.

So the main thing I'd like to ask @abravalheri is whether you think it's worth doing the method I outlined above, or do you think the current implementation is sufficient?

Probably both proposals have their merit (thank you very much for investigating them). I believe that build_type_information would the most "semantically elegant" because it stops considering type information files as "data-files". However, as you mentioned the inclusion of the files would start to be non-optional... Therefore the approach with data-files seems to be the one that leaves a escape hatch for users to opt-out (and it is "backwards compatible" with the way users have been approaching the problem historically in setuptools).

We can give it a try to the implementation you have already submitted and see how it goes. In the first stage we can make sure the feature is documented as "beta" (both in the docs and in the changelog), after some months of experimentation we can promote out of beta. What do you think?

[abravalheri]

Regarding the CI errors, I believe you can simply rebase in the latest main.

[Danie-1]

I've pushed a commit to fix the tests on windows.
I also realised I forgot to respond to your question about documenting the feature as beta. That sounds like a good idea to me. For the docs, I suppose there should be something on the data files page mentioning it. Is that the only page where it would need mentioning?

[abravalheri]

Sorry for the delay and thank you very much @Danie-1

For the docs, I suppose there should be something on the data files page mentioning it. Is that the only page where it would need mentioning?

I added something quick in 111b05f. Please feel free to improve.

[abravalheri]

I think this is good enough for trying out.

@Danie-1 please let me know if you want to do any other changes, otherwise I think we can merge, so it can be included in the next release (not sure if it is going to happen next week, maybe in the beginning of November).

[Danie-1]

That sounds like a good plan to me. I don't have any suggestions for changes, other than a typo fix that I've just pushed.

[abravalheri]

Thank you very much @Danie-1.

[zhegata]

We just ran into the issue that .pyi files are not added to our packages by default. It would be relatively easy to handle on our side, but I'm glad to see this has already been solved out of the box (and looks like right on time for us). Thank you for this.

Do you have an idea when this will be released?

[abravalheri]

Hi @zhegata please have a look on the comment above: #4021 (comment)