markdown: Accept legacy Pattern in inlinePatterns registry
#11027
Conversation
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| @@ -21,7 +21,7 @@ class _ReadableStream(Protocol): | |||
|
|
|||
| class Markdown: | |||
| preprocessors: Registry[preprocessors.Preprocessor] | |||
| inlinePatterns: Registry[inlinepatterns.InlineProcessor] | |||
| inlinePatterns: Registry[inlinepatterns.Pattern] | |||
There was a problem hiding this comment.
This is different to the annotations the markdown library now has inline:
- https://github.com/Python-Markdown/markdown/blob/5d7f2559c31844f788ea41132f40ace7555518e6/markdown/core.py#L154
- https://github.com/Python-Markdown/markdown/blob/5d7f2559c31844f788ea41132f40ace7555518e6/markdown/inlinepatterns.py#L53
Cc. @oprypin, who recently added this type hint in typeshed
There was a problem hiding this comment.
It is my mistake, actually this PR is more correct 👍
I was adding this both upstream and in typeshed btw.
| @@ -21,7 +21,7 @@ class _ReadableStream(Protocol): | |||
|
|
|||
| class Markdown: | |||
| preprocessors: Registry[preprocessors.Preprocessor] | |||
| inlinePatterns: Registry[inlinepatterns.InlineProcessor] | |||
| inlinePatterns: Registry[inlinepatterns.Pattern] | |||
There was a problem hiding this comment.
It is my mistake, actually this PR is more correct 👍
I was adding this both upstream and in typeshed btw.
| @@ -6,7 +6,7 @@ from xml.etree.ElementTree import Element | |||
| from markdown import util | |||
| from markdown.core import Markdown | |||
|
|
|||
| def build_inlinepatterns(md: Markdown, **kwargs) -> util.Registry[InlineProcessor]: ... | |||
| def build_inlinepatterns(md: Markdown, **kwargs) -> util.Registry[Pattern]: ... | |||
There was a problem hiding this comment.
InlineProcessor is a subclass of Pattern and everywhere else the more general class is correct. However this particular method really does create a registry of only InlineProcessor. If the covariance stuff all works out, I believe we should leave this part as is. I'll try later today what mypy thinks about it with the full codebase
| def build_inlinepatterns(md: Markdown, **kwargs) -> util.Registry[Pattern]: ... | |
| def build_inlinepatterns(md: Markdown, **kwargs) -> util.Registry[InlineProcessor]: ... |
There was a problem hiding this comment.
A Registry can’t be covariant because it’s mutable. That is, we can’t convert reg1: Registry[InlineProcessor] to reg2: Registry[Pattern] because then one could add a Pattern to reg2 and read it out from reg1 as an InlineProcessor even if it isn’t.
So if this is going to be used as Registry[Pattern], it needs to be declared that way from the start.
|
Related pull request: I'll need to update my pull request after this one is merged. I'd say let's merge this one first. |
No description provided.