Skip to content

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

Description

@not-my-profile

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).

Activity

  1. changed the title [-]Pygments type stubs are broken[/-] [+]Pygments `Formatter.__init__` overloads do not work as intended[/+] on Mar 4, 2022
  2. srittau commented on Mar 4, 2022

    @srittau
    Collaborator

    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.

  3. AlexWaygood commented on Mar 4, 2022

    @AlexWaygood
    Member

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

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

  4. not-my-profile commented on Mar 4, 2022

    @not-my-profile
    ContributorAuthor

    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.

  5. AlexWaygood commented on Mar 4, 2022

    @AlexWaygood
    Member

    Cc. @Dreamsorcerer, who authored #6819

  6. AlexWaygood commented on Mar 4, 2022

    @AlexWaygood
    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
  7. AlexWaygood commented on Mar 4, 2022

    @AlexWaygood
    Member

    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
  8. Dreamsorcerer commented on Mar 4, 2022

    @Dreamsorcerer
    Contributor

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

  9. not-my-profile commented on Mar 4, 2022

    @not-my-profile
    ContributorAuthor

    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.

  10. AlexWaygood commented on Mar 4, 2022

    @AlexWaygood
    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions