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

The current py.typed setup does not seem ideal #3413

Open
Andrew-S-Rosen opened this issue Oct 18, 2023 · 8 comments
Open

The current py.typed setup does not seem ideal #3413

Andrew-S-Rosen opened this issue Oct 18, 2023 · 8 comments

Comments

@Andrew-S-Rosen
Copy link
Member

Problem

I am using mypy on a package with Pymatgen as a dependency and am getting back the following even though there is a py.typed marker in the base pymatgen directory. Any insights?

schemas/_aliases/emmet.py:11: error: Skipping analyzing "pymatgen.core.composition": module is installed, but missing library stubs or py.typed marker  [import]
schemas/_aliases/emmet.py:12: error: Skipping analyzing "pymatgen.core.lattice": module is installed, but missing library stubs or py.typed marker  [import]
schemas/_aliases/emmet.py:13: error: Skipping analyzing "pymatgen.core.periodic_table": module is installed, but missing library stubs or py.typed marker  [import]
schemas/_aliases/emmet.py:14: error: Skipping analyzing "pymatgen.core.structure": module is installed, but missing library stubs or py.typed marker  [import]
schemas/_aliases/emmet.py:15: error: Skipping analyzing "pymatgen.entries.computed_entries": module is installed, but missing library stubs or py.typed marker  [import]
schemas/_aliases/emmet.py:16: error: Skipping analyzing "pymatgen.io.vasp.inputs": module is installed, but missing library stubs or py.typed marker  [import]

Proposed Solution

Confirm py.typed marker carries over to the full package.

Alternatives

No response

@janosh
Copy link
Member

janosh commented Oct 18, 2023

Looks like it's not being shipped despite being included (incorrectly?) as package_data in setup.py

"pymatgen": ["py.typed"],

@janosh
Copy link
Member

janosh commented Oct 18, 2023

Maybe it wouldn't work with a single py.typed even if shipped successfully since we're using namespace packages?

For namespace packages (see PEP 420), the py.typed file should be in the submodules of the namespace, to avoid conflicts and for clarity.

@Andrew-S-Rosen
Copy link
Member Author

Very good points that I don't know the answers to!

@Andrew-S-Rosen Andrew-S-Rosen changed the title Is the current py.typed setup ideal? The current py.typed setup does not seem ideal Oct 19, 2023
@janosh
Copy link
Member

janosh commented Nov 20, 2023

Potentially relevant: setuptools v69.0.0 release

Include type information (py.typed, *.pyi) by default (pypa/setuptools#3136) -- by @Danie-1, EXPERIMENTAL. (pypa/setuptools#3136)

@Andrew-S-Rosen
Copy link
Member Author

@janosh: Is it implying that if you're using setuptools, you don't need a py.typed at all? That'd be nice because it's so ugly lol

@janosh
Copy link
Member

janosh commented Nov 20, 2023

I think pypa/setuptools#3136 just added a new convenience option include_typing_files = True which we would add in setup.py to make sure we package py.typed. We still need that file it just avoids user error in trying to include that file.

@Andrew-S-Rosen
Copy link
Member Author

Ah, I see. That makes sense.

@DanielYang59
Copy link
Contributor

DanielYang59 commented Nov 25, 2024

I just came across this issue randomly, I believe #4018 (migrated to #4199 now) would fix this to align with PEP 420 (i.e. add py.typed marker file for each namespace subpackage), let me know if I'm missing anything :)


I think pypa/setuptools#3136 just added a new convenience option include_typing_files = True which we would add in setup.py to make sure we package py.typed

Looks like that PR original proposed to add this include_typing_files but ended up Include py.typed and *.pyi files by default., so I guess we don't need this anymore? Also tested locally with #4018, both binary and source distributions do have py.typed

My personal opinion (which might not be shared by the setuptools maintainers) is that it is very unlikely that a new configuration option will be added. There is already a lot of options and people seem to have a hard time understanding/dealing with them.

Something that is more likely to happen would besetuptools starting to includ *.pyi files by default.

Oh yeah, I forgot about py.typed. I think including both by default would be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants