-
Notifications
You must be signed in to change notification settings - Fork 865
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
Full type annotations #1389
Comments
I am indifferent to this. I was willing to add type annotations for documentation purposes because I see the value. However, I see no value in type annotations for annotations sake. Probably because I'm an old-school Python dev who never had any annotations for many years, and well, they don't actually do anything... that is unless you use some tools that I don't use. The point is that to me they are just noise in my code. But as that noise now has value for the documentation, I am willing to overlook it. However, if the noise isn't going to contribute to the documentation, then it's just an annoyance. Which means I am not likely to maintain and update it if/when the code changes. But if someone else wants to do the work, then they are certainly welcome to. I suppose that was a long-winded way of saying a PR is welcome. But I'm not putting any more effort into this personally. |
I understand, thanks for your answer :) I have no plans to work on this myself in the short term, but it's good to know you'd accept PRs and maintenance! |
A lot happened in other PRs since this issue was last updated. Of significance is this comment of mine where I laid out more specific direction about my expectations regarding type annotations and whether this library should be fully type checked. In the ensuing discussion and updates to the PR it became clear that the type checker being used (mypy) could not be configured to meet my expectations (it was too strict) and the PR was closed as rejected. In that same discssion I became convinced that it was also impossible for this library to be type complete (have a py.typed file). However, I have continued to question if that was correct. So today, I did some more research and found this typing documentation (linked from the typeshed project) which, among other things, spells out what constitutes type completeness (see also the earlier section How much of my library needs types?. Significantly, my review of this document does not seem to suggest that my previous position was off the mark. Noteworthy among the things listed there are the following:
Private objects do not need to be annotated. Yet, in the proposed PRs I repeatedly rebuffed attempts to annotate private objects. This is especially noted in the cases were a local variable name was reused within a function/method for a different type. There is no need to annotate these at all and a type checker should not be checking them. Of particular interest to me was the following section:
There were plenty of other items in that list, but the one included above became a constant point of contention. In any event, after reviewing the linked document, I believe that this library is very nearly type complete. With perhaps a couple isolated minor adjustments (and possibly a few ignore comments), we should be able to say our code is type complete and include a The problem is finding a type checker which can be configured to check the minimum requirements only and working out what that configuration is. I would be open to reviewing a PR which aimed to accomplish that. However, I would expect the PR to make as few changes to the code as possible. The changes should primarily be to annotations and/or the type checker's configuration. I would even prefer if any existing annotations which are not required by the cited documentation (such as on private objects) be removed; although, they can be left there now that they exist in the existing code base. |
Now that you added type annotations in many places, I was wondering if you would consider annotating the whole package, and mark it as typed with a
py.typed
file? For now users who want to type-check their code using Python-Markdown can installtypes-markdown
which comes from typeshed.The text was updated successfully, but these errors were encountered: