8000 Pygments `Formatter.__init__` overloads do not work as intended · Issue #7436 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Pygments Formatter.__init__ overloads do not work as intended #7436

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.

Sign up for GitHub

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

Jump to bottom
Open
not-my-profile opened this issue Mar 4, 2022 · 9 comments
8000
Open

Pygments Formatter.__init__ overloads do not work as intended #7436

not-my-profile opened this issue Mar 4, 2022 · 9 comments

Comments

@not-my-profile
Copy link
Contributor
not-my-profile commented Mar 4, 2022 •
edited
Loading

In #6819 pygments.formatter.Formatter was made generic to improve the type safety:

_T = TypeVar("_T", str, bytes)

class Formatter(Generic[_T]):
    @overload
    def __init__(self: Formatter[str], *, encoding: None = ..., outencoding: None = ..., **options) -> None: ...
    @overload
    def __init__(self: Formatter[bytes], *, encoding: str, outencoding: None = ..., **options) -> None: ...
    @overload
    def __init__(self: Formatter[bytes], *, encoding: None = ..., outencoding: str, **options) -> None: ...

Apparently this was never tested, since it does not actually work:

from pygments.formatters.html import HtmlFormatter

reveal_type(HtmlFormatter())
reveal_type(HtmlFormatter(encoding='utf-8'))
$ mypy foo.py
foo.py:3: note: Revealed type is "pygments.formatters.html.HtmlFormatter[builtins.str*]"
foo.py:4: note: Revealed type is "pygments.formatters.html.HtmlFormatter[builtins.str*]"
$ pyright foo.py
  /tmp/foo.py:3:13 - information: Type of "HtmlFormatter(encoding='utf-8')" is "HtmlFormatter[Unknown]"
  /tmp/foo.py:4:13 - information: Type of "HtmlFormatter()" is "HtmlFormatter[Unknown]"

The intention of the overloads is that HtmlFormatter(encoding='utf-8') should be of type HtmlFormatter[builtins.bytes].

I am not sure if this is a mypy bug or if such overload inference should even work in the first place (given that pyright also infers the type variable to be Unknown: microsoft/pyright#3146).
@not-my-profile not-my-profile mentioned this issue Mar 4, 2022
Closed
@not-my-profile not-my-profile changed the title Pygments type stubs are broken Pygments Formatter.__init__ overloads do not work as intended Mar 4, 2022
@srittau
Copy link
Collaborator
srittau commented Mar 4, 2022

The problem is most likely that HtmlFormatter is not explicitly generic and does not override __init__, which Formatter.__init__() annotates self with Formatter[...]. Maybe that's something PEP 673 can help to fix, but I'm not sure. In the meantime, we should copy Formatter.__init__() to HtmlFormatter.

@AlexWaygood
Copy link
8000
Member

Maybe that's something PEP 673 can help to fix, but I'm not sure.

It probably ultimately needs python/typing#548, right?

@not-my-profile
Copy link
Contributor Author

In the meantime, we should copy Formatter.__init__() to HtmlFormatter.

I'd be in favor of completely reverting #6819 instead because the following should type check with strict settings:

class Foo(pygments.formatter.Formatter):
    pass

but currently fails with Missing type parameters for generic type "Formatter".

And doing a dance like

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    _Base = pygments.formatter.Formatter[str]
else:
    _Base = pygments.formatter.Formatter

class Foo(_Base):
    pass

is IMHO completely ridiculous.

I attempted to upstream the generic base class to pygments but they're not interested (pygments/pygments#2081).

So I'd say revert #6819 till there is something like python/typing#956.

@AlexWaygood
Copy link
Member

Cc. @Dreamsorcerer, who authored #6819

@not-my-profile not-my-profile mentioned this issue Mar 4, 2022
Open
@AlexWaygood
Copy link
Member

[D]oing a dance like

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    _Base = pygments.formatter.Formatter[str]
else:
    _Base = pygments.formatter.Formatter

class Foo(_Base):
    pass

is IMHO completely ridiculous.

Here's an alternative hack:

from pygments.formatter import Formatter
from types import GenericAlias

Formatter.__class_getitem__ = classmethod(GenericAlias)  # type: ignore[attr-defined]

class Foo(Formatter[str]):
    pass

@AlexWaygood
Copy link
Member
AlexWaygood commented Mar 4, 2022 •
edited
Loading

If you don't care about runtime introspection, you don't even need the extra imports from types. You can just do:

from pygments.formatter import Formatter

Formatter.__class_getitem__ = classmethod(lambda cls, val: cls) # type: ignore[attr-defined]

class Foo(Formatter[str]):
    pass

Don't like the # type: ignore? Just use setattr instead ;)

from pygments.formatter import Formatter

setattr(Formatter, "__class_getitem__", classmethod(lambda cls, val: cls))

class Foo(Formatter[str]):
    pass

@Dreamsorcerer
Copy link
Contributor

I don't remember the details of the implementation, but this is what it fixed for me:
aio-libs/aiohttp-devtools@3405752

@not-my-profile
Copy link
Contributor Author

Here's an alternative hack

IMHO using stubs from typeshed shouldn't force you to monkey patch the API on runtime just to create a subclass.

@AlexWaygood
Copy link
Member

Here's an alternative hack

IMHO using stubs from typeshed shouldn't force you to monkey patch the API on runtime just to create a subclass.

Of course not. But this is a known problem that's been around for a while, and there's no easy solution here. Just trying to provide workarounds :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@srittau @Dreamsorcerer @AlexWaygood @not-my-profile
0