Skip to content

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Oct 25, 2023

The origins of these are three-fold:

  • Merging in stubs from https://github.com/python/typeshed/tree/main/stubs/Markdown using "merge-pyi"
    • Note: we can consider these annotations to be the important ones because it's what people have been adding according to their own need
  • Double-checking around places where stubs were already added from the above, particularly conflicts with annotations that got added in this repository already
    • Taking the opportunity to declare a generic "Registry of T" class
  • Running "mypy" and eliminating the most glaring errors it reported
The origins of these are three-fold: * Merging in stubs from https://github.com/python/typeshed/tree/main/stubs/Markdown using "merge-pyi" - Note: we can consider these annotations to be the important ones because it's what people have been adding according to their own need * Double-checking around places where stubs were already added from the above, particularly conflicts with annotations that got added in this repository already + Taking the opportunity to declare a generic "Registry of T" class * Running mypy and eliminating the most glaring errors it reported
@waylan
Copy link
Member

waylan commented Oct 26, 2023

So there appears to be a couple code changes included in this (I don't mean adjustments to whitespace/line length). Could you please remove those? They should be submitted separately.

@oprypin
Copy link
Contributor Author

oprypin commented Oct 26, 2023

I only recognized one change that's clearly not about type annotations and reverted it.

I'm not sure if you also mean the NamedTuple change (which I think is on topic, as it's just the way to spell the same thing but with type annotations) or if there was something else.

@waylan
Copy link
Member

waylan commented Oct 26, 2023

I was thinking of the NameTuples, but you are correct, they do have type annotations. So nevermind on that.

@waylan
Copy link
Member

waylan commented Oct 26, 2023

Thanks for this. Some of the wrong annotations I completely missed.

I'll merge this after a changelog entry is added.

@oprypin
Copy link
Contributor Author

oprypin commented Oct 27, 2023

That is done

@oprypin
Copy link
Contributor Author

oprypin commented Oct 27, 2023

For future reference:

  1. The main reason that nobody complained about the current type annotations is probably that they aren't actually used by type checkers. Unless a file named py.typed exists, the dominant type checker "mypy" ignores the types and asks you to install https://pypi.org/project/types-Markdown/ anyway.

  2. If you are interested in a fully correct type-checked codebase (according to "mypy") as well as official external-facing type annotations support (by adding py.typed), let me know anytime, I can take up that task and help maintain this going forward. Otherwise that's all for now.

@waylan
Copy link
Member

waylan commented Oct 30, 2023

Note that you have a conflict with the changelog (presumably with #1392).

Regarding a fully correct type-checked codebase (according to "mypy") that is probably a reasonable goal. Personally, I haven't really explored what's involved given that this is a rather new thing for Python. So contributions are welcome.

@oprypin
Copy link
Contributor Author

oprypin commented Oct 30, 2023

Note that you have a conflict with the changelog

Hm? I had already merged to resolve the conflict. There was just the spelling check failing

Regarding a fully correct type-checked codebase (according to "mypy") that is probably a reasonable goal.

Great, so I'll continue looking into this soon (in a separate PR)

@waylan waylan merged commit 99425b4 into Python-Markdown:master Oct 30, 2023
@pawamoy
Copy link
Contributor

pawamoy commented Oct 30, 2023

This PR almost closes #1389 🙂

@oprypin
Copy link
Contributor Author

oprypin commented Oct 30, 2023

Oh I didn't see that!
No there are still like 100 mypy errors so maybe not "almost" 😅

@oprypin oprypin deleted the annot branch October 30, 2023 19:32
@pawamoy
Copy link
Contributor

pawamoy commented Oct 30, 2023

Oh OK I thought there was just the py.typed file missing 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants