Skip to content
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

Init file in stubs-only package #108

Open
gleb-chipiga opened this issue Jan 12, 2022 · 20 comments
Open

Init file in stubs-only package #108

gleb-chipiga opened this issue Jan 12, 2022 · 20 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@gleb-chipiga
Copy link

Project structure:

aiojobs-stubs
└── aiojobs-stubs
    └── __init__.pyi
└── aiojobs_protocols
    └── __init__.py
    └── py.typed
└── LICENSE
└── MANIFEST.in
└── setup.py

setup.py

setup(
    ...
    package_data={'aiojobs-stubs': ['__init__.pyi'],
                  'aiojobs_protocols': ['py.typed']},
    packages=['aiojobs-stubs', 'aiojobs_protocols'],
    python_requires='>=3.8'
)

MANIFEST.in

include LICENSE

Run command:

python -m build

Receive message:

package init file 'aiojobs-stubs/__init__.py' not found (or not a regular file)

According to PEP 561, the __init__.py file is not needed in such a package.

@henryiii
Copy link

henryiii commented Jan 12, 2022

Folder name should be aiojobs_stubs not aiojobs-stubs (must be importable). The outer folder name can be anything, but the inner one must be importable.

(I was corrected later, but to clarify this is wrong, it is intentionally unimportable at runtime)

@layday
Copy link
Member

layday commented Jan 12, 2022

aiojobs-stubs is correct per PEP 561. This message is from setuptools (or distutils) and AFAICT it doesn't cause a failure. I suppose setuptools expects you to declare it as a namespace package because it lacks an __init__.py. You might want to ask on their tracker.

@layday
Copy link
Member

layday commented Jan 12, 2022

@abravalheri might know :)

@abravalheri
Copy link
Contributor

Hi @gleb-chipiga, I confirm that I see the warning, but everything builds fine (so hopefully you can carry on your work without much trouble).

These are the commands I used to replicate:

% cd /tmp
% mkdir -p test_aiojobs_stubs
% cd test_aiojobs_stubs
% mkdir aiojobs-stubs aiojobs_protocols
% touch aiojobs-stubs/__init__.pyi aiojobs_protocols/{__init__.py,py.typed} LICENSE
% echo 'include LICENSE' > MANIFEST.in
% echo "from setuptools import setup
setup(
name='aiojobs-stubs',
package_data={'aiojobs-stubs': ['__init__.pyi'],
              'aiojobs_protocols': ['py.typed']},
packages=['aiojobs-stubs', 'aiojobs_protocols'],
python_requires='>=3.8'
)" > setup.py
% virtualenv .venv -p py38
% .venv/bin/python -m pip install -U pip build
% .venv/bin/python -m build
% unzip -l dist/aiojobs_stubs-0.0.0-py3-none-any.whl
Archive:  dist/aiojobs_stubs-0.0.0-py3-none-any.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2022-01-12 18:19   aiojobs-stubs/__init__.pyi
        0  2022-01-12 18:19   aiojobs_protocols/__init__.py
        0  2022-01-12 18:19   aiojobs_protocols/py.typed
        0  2022-01-12 18:28   aiojobs_stubs-0.0.0.dist-info/LICENSE
      183  2022-01-12 18:28   aiojobs_stubs-0.0.0.dist-info/METADATA
       92  2022-01-12 18:28   aiojobs_stubs-0.0.0.dist-info/WHEEL
       32  2022-01-12 18:28   aiojobs_stubs-0.0.0.dist-info/top_level.txt
      655  2022-01-12 18:28   aiojobs_stubs-0.0.0.dist-info/RECORD
---------                     -------
      962                     8 files

This is caused by the implementation of pypa/distutils that predates PEP 561.
(Ideally setuptools should also include *.pyi automatically).

This issue could be reported both in pypa/setuptools (FR for automatically adding *.pyi) and pypa/distutils (remove the warn if all the files inside the folder are *.pyi).

@henryiii
Copy link

Ahh, I hadn't thought about that before, but it makes sense.

Shouldn't pypa/distutils or pypa/setuptools also check for __init__.pyi and avoid the warning for that case too?

Edit: Actually, that's what you suggest at the end, yes.

@henryiii
Copy link

Should this issue be moved? I can move it to packaging-problems, where it might get more views, or someone else could move it to distutils or setuptools.

@abravalheri
Copy link
Contributor

I think GitHub only allow maintainers of both the "origin" and the "destination" repository to move the issue 😅.

If you think packaging-problems is the most appropriate place, maybe that is a good solution (I honestly don't know what is the best approach).

@henryiii
Copy link

I think GitHub only allow maintainers of both the "origin" and the "destination"

Yes, that's why I'm limited to moving it to packaging-problems. :) I think new issues could be opened for distutils (adding pyi to the check) and setuptools (adding .pyi files, and py.typed files to the default files) would be a good idea. I'm not sure what the policy / ideal situation is for changing distutils, due to it still existing in the stdlib too.

@layday

This comment has been minimized.

@FFY00 FFY00 pinned this issue Jan 12, 2022
@FFY00 FFY00 unpinned this issue Jan 12, 2022
@FFY00 FFY00 transferred this issue from pypa/build Jan 12, 2022
@gleb-chipiga
Copy link
Author

setup.py gives no such warning. I only get it when using build.

from setuptools import setup

Run command:

setup.py sdist bdist_wheel

@henryiii
Copy link

I don't think the setup.py behavior is very important, though it is interesting to note:

https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html

@henryiii
Copy link

The pip wheel . behavior would be interesting, though.

@layday
Copy link
Member

layday commented Jan 12, 2022

I can reproduce it with setup.py so I'm not sure why you can't.

@gleb-chipiga
Copy link
Author

pip wheel . worked without warning.

@gleb-chipiga
Copy link
Author

Python 3.9.9

build==0.7.0
setuptools==60.5.0
pip==21.3.1
wheel==0.37.1

@layday
Copy link
Member

layday commented Jan 12, 2022

(venv) ~/D/t/aiojobs-stubs (main|…) $ python -V
Python 3.9.9
(venv) ~/D/t/aiojobs-stubs (main|…) $ python -m pip list
Package    Version
---------- -------
pip        21.3.1
setuptools 60.5.0
wheel      0.37.1
(venv) ~/D/t/aiojobs-stubs (main|…) $ python setup.py sdist bdist_wheel | grep package
package init file 'aiojobs-stubs/__init__.py' not found (or not a regular file)

@gleb-chipiga
Copy link
Author

There is indeed a warning. I didn't notice it. Sorry.

@FFY00
Copy link
Member

FFY00 commented Jan 13, 2022

I'm not sure what the policy / ideal situation is for changing distutils, due to it still existing in the stdlib too.

I only skimmed the issue, so did not see this. But distutils in ths stdlib should not receive any updates, the development has been moved to this repo, which is periodically synced with the local setuptools copy, and will eventually be merged there.

@ethanhs
Copy link

ethanhs commented Nov 3, 2022

Folder name should be aiojobs_stubs not aiojobs-stubs (must be importable). The outer folder name can be anything, but the inner one must be importable.

@henryiii I wanted to clarify that this is incorrect, one of the goals of the -stubs suffix is to make the folder non-importable, it exists solely for static analysis purposes.

(I mention this since someone came across your comment and was confused, so you may want to edit it and either clarify or point to this comment)

@henryiii
Copy link

henryiii commented Nov 3, 2022

OK, I fixed it, though you only have to scroll down a couple of comments to see that it was an incorrect statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants