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

Full type annotations #1389

Open
pawamoy opened this issue Oct 9, 2023 · 3 comments
Open

Full type annotations #1389

pawamoy opened this issue Oct 9, 2023 · 3 comments
Labels
someday-maybe Approved low priority request.

Comments

@pawamoy
Copy link
Contributor

pawamoy commented Oct 9, 2023

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 install types-markdown which comes from typeshed.

@waylan
Copy link
Member

waylan commented Oct 9, 2023

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.

@waylan waylan added the someday-maybe Approved low priority request. label Oct 9, 2023
@pawamoy
Copy link
Contributor Author

pawamoy commented Oct 9, 2023

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!

@waylan
Copy link
Member

waylan commented Nov 8, 2023

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:

  • Symbols whose names begin with an underscore (but are not dunder names) are considered private.
    ...
  • Local variables within a function (including nested functions) are always considered private.

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:

Type annotations can be omitted in a few specific cases where the type is obvious from the context:
...

  • The return type for an __init__ method does not need to be specified, since it is always None.

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 py.typed file.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
someday-maybe Approved low priority request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants