-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Make package PEP 561 compatible #28831
Conversation
Cool thanks for submitting this - makes sense to do in the long run. Any chance you have personal experience doing this on other projects? My main concern would be the maturity of annotations; we have them but they aren't necessarily comprehensive yet. That may or may not be a blocker to doing this just curious if you or anyone you know has experience to know the pros and cons of putting these out there |
@simonjayhawkins might have thoughts as well |
I think that a bigger problem is that Pandas doesn't type check at the moment. Right now (bee17d5) there are 55 errors. Some are easy fix (for example #28843, or some other minor fixes I am working on), other might be much harder to tackle (there is a bunch of inheritance issues, which are real pain). Nonetheless I'd say that, conditioning on fixing or type-ignoring aforementioned problem exposing (even partial) annotations is useful (please bear in mind that I am not objective, as lack of Pandas annotations is blocker for my own work), as long as individual modules (not necessarily all) provide complete, even if mostly dynamic annotations. |
We do type check with mypy as part of our CI - are those failures accounting for what's in our |
I am running Mypy directly with
The result is
(difference in numbers accounts for things I fixed locally, sorry for that). As far as I can tell none of these is directly caused by missing annotations. Some are just incompatible (created by MonkeyType?), other are small correctness issues or already mentioned inheritance problems. Not sure about the rest yet, I just started reviewing these. |
@c4f3a0ce if you merge master the Linux py36_32bit build should pass |
The mypy errors reported above are not a blocker for this PR. Is this something we want to do at this early stage? see #28843 (comment) onwards. |
I could be convinced otherwise but I think this is premature. I'd rather annotate the entire API internally first before release to public |
i agree we are not ready to release typing even for external dev use |
Pandas has growing number of type annotations but these are not properly advertised to type checking tools. According to PEP 561 packages providing annotations should contain
py.typed
files in corresponding packages.py.typed
is applied recursively, if placed in the root, and can accept missing annotations, if containspartial\n
.This PR adds such file and includes it in
package_data
.black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff