Pygments: fix get_style_by_name return type

[nwalters512]

As seen in the example from https://pygments.org/docs/styles/, get_style_by_name returns a class, not an instance:

>>> from pygments.styles import get_style_by_name
>>> get_style_by_name('colorful')
<class 'pygments.styles.colorful.ColorfulStyle'>

[AlexWaygood]

Thanks!

mypy_primer has a point here

[AlexWaygood]

Hmm, the porcupine hit is a little unfortunate (code here: https://github.com/Akuli/porcupine/blob/345fc99f4aee9f737faddd523d28f9157f3fb99c/porcupine/plugins/longlinemarker.py#L57). It seems that with the current annotation -- where the function is, in effect, declared as returning an "instance of the metaclass" -- mypy recognises that objects returned by this function are iterable. But if we make this proposed change, mypy starts emitting false-positive errors if you try to iterate over the returned object. This is due to a mypy limitation in its understanding of metaclasses, but it still seems a shame to start introducing false-positive errors here, given that the current annotation isn't actually wrong.

It's interesting that you say in PrairieLearn/PrairieLearn#7185 (comment) that this fixes a pyright issue in that PR -- I thought type checkers were generally pretty grumpy with you if you had a dynamic base class of any kind, anywhere.

[AlexWaygood]

(Cc. @Akuli, who contributed these stubs and is also the maintainer of porcupine.)

[Akuli]

What is the reason for changing this? What is the practical problem that this PR solves?

[nwalters512]

@Akuli This PR makes the type information reflect what actually happens at runtime, which is that get_style_by_name returns a type, not an instance of that type. This is useful if I want to use that type for something else (e.g. to inherit the returned Style class).

[JelleZijlstra]

it is annotated as returning an instance of StyleMeta though, which is (approximately) the same as returning type[Style]. (I glanced over this the first time around and thought it returned Style, which would have been incorrect.)

needs more thought

[Akuli]

get_style_by_name returns a type, not an instance of that type.

The old annotation is correct, because the returned type is an instance of StyleMeta:

>>> from pygments.styles import get_style_by_name
>>> type(get_style_by_name('colorful'))
<class 'pygments.style.StyleMeta'>
>>> from pygments.style import StyleMeta
>>> isinstance(get_style_by_name('colorful'), StyleMeta)
True

It seems to me that this PR introduces one practical problem without solving other practical problems, while from a theorethical point being equally correct as the old annotation.

[Akuli]

Sorry, I somehow missed that you mentioned inheriting from the returned object. It is going to be annoying for someone either way: if we return type[Style] then I cannot iterate the object, and if we return StyleMeta then you cannot subclass the object.

I'm fine with merging this, and equally fine with not merging this.

[nwalters512]

Hmm, I'm admittedly not a Python expert. Is the issue I'm hitting when trying to treat the return value as a type a limitation of type checkers, or the types, or something else? Or is Pygments trying to be overly clever by making something both a class and an iterable?

[JelleZijlstra]

I think these are just type checker limitations. As I understand it, the current annotations will make it so Pyright doesn't let you inherit from the returned class, and the PR's annotations will make it so that mypy doesn't let you iterate over the returned class even though it is iterable. Neither issue is obviously worse than the other, so I'd prefer to stick with the current annotations so we don't introduce churn for users.

If mypy or pyright change their behavior here, we can reconsider.

[Akuli]

To me, it is both pygments being overly clever, and limitations of type checkers. Ideally libraries wouldn't try to make classes iterable :)

Maybe the correct solution would be to make mypy recognize type[Foo] as iterable when Foo is a class whose metaclass defines __iter__. Then my code would also type-check correctly with the annotation from this PR.

[AlexWaygood]

Maybe the correct solution would be to make mypy recognize type[Foo] as iterable when Foo is a class whose metaclass defines __iter__. Then my code would also type-check correctly with the annotation from this PR.

This might already be implemented on mypy master actually, via:

• Support protocol inference for Type[T] via metaclass mypy#14554

[Akuli]

It is fixed on mypy master. Let's wait for the next mypy release and then merge :)

I checked locally by mypying this script:

from typing import Iterator

class StyleMeta(type):
    def __iter__(self) -> Iterator[str]:
        yield 'a'
        yield 'b'
        yield 'c'

class Style(metaclass=StyleMeta):
    pass

x: type[Style] = Style
lel = iter(x)

On mypy's release-1.0 branch this errors on the last line, but on master it succeeds.

[AlexWaygood]

The next mypy release should hopefully be coming out in the next week or so:

• Release 1.1 planning mypy#14730

[Akuli]

Closing and re-opening to run mypy_primer again.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

sphinx (https://github.com/sphinx-doc/sphinx)
- sphinx/highlighting.py:114:20: error: Incompatible return value type (got "StyleMeta", expected "Style")  [return-value]
+ sphinx/highlighting.py:114:20: error: Incompatible return value type (got "Type[Style]", expected "Style")  [return-value]