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

Fix: Add py.typed to root package #666

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

whardier
Copy link

I don't believe I need to add this to the pypika.clickhouse package. Works well in vscode and along with mypy.

@whardier whardier requested a review from a team as a code owner January 31, 2022 18:33
@whardier
Copy link
Author

Referencing #609
Referencing #323
Referencing #410, #413, #414, #418

@AzisK
Copy link

AzisK commented Sep 25, 2023

Is this still needed?

@whardier
Copy link
Author

All Python packages should attempt to be properly typed or provide a stubs package that can be used in tandem with their product. See https://peps.python.org/pep-0561/#packaging-type-information ... this helps a LOT with static and runtime type inspection, evaluation, as well as vulnerability fingerprinting and can catch issues well before code using a specific library is committed.

I've attempted to reach out to Kayak through several contacts to see if this is something we could help maintain within this library and not had any response until your comment today.

@AzisK
Copy link

AzisK commented Sep 29, 2023

I am happy to have responded. We will improve its typing.

Having said that and partly read the PEP, I still don't have a clear and good vision how this one file addition solves or helps solve everything but I trust your technical judgement based on the severity of your comments as well as involvement with this repo. Could you elaborate or showcase it with some examples? It would definitely be beneficial to me but since these pull requests are open source, it would be beneficial for someone else inexperienced in this as well.

I asked if it is still needed because I though that one can do this or type-hint all of the methods. I have looked at the referenced pull requests and they seem to have been merged already. However, from your comment I understand that that is not enough and this is still needed. Am I right about it?

P.S. Please pull the newest code from master so that the CI tests kick in

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 this pull request may close these issues.

None yet

2 participants