Fix type errors in Pyright

[layday]

Summary

MyPy treats the user-declared MYPY constant equivalently
to typing.TYPE_CHECKING (see https://mypy.readthedocs.io/en/stable/more_types.html#conditional-overloads).
This can be used to trick other type checkers into ignoring
MyPy-specific types.

AttrsInstance needs to be declared in a separate stub file so as to
not export MYPY.

Pull Request Check List

• Added tests for changed code.
  Our CI fails if coverage is not 100%.
• New features have been added to our Hypothesis testing strategy.
• Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
  • ...and used in the stub test file tests/typing_example.py.
  • If they've been added to attr/__init__.pyi, they've also been re-imported in attrs/__init__.pyi.
• Updated documentation for changed code.
  • New functions/classes have to be added to docs/api.rst by hand.
  • Changes to the signature of @attr.s() have to be added by hand too.
  • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
    Find the appropriate next version in our __init__.py file.
• Documentation in .rst files is written using semantic newlines.
• Changes (and possible deprecations) have news fragments in changelog.d.
• Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

[hynek]

E     E: ...ed type is "Type[attr.AttrsInstance]"
E     A: ...ed type is "Type[attr._typing_compat.AttrsInstance]"

Hm is there a way to get rid of the namespace? Otherwise you'll have to do it inline I'm afraid

[layday]

The problem with that is that Pyright solves <a conditionally defined protocol> | Any alias to <a conditionally defined protocol>. If we use an annotated variable instead (AttrsInstance: Any), Pyright (rightly) complains that AttrsInstance is being redefined. So, if we were to move it into __init__.py, we'd be forced to mangle the name of the protocol, since we won't be able to perform the little reassignment dance that we are doing now. Or we could ignore the Pyright error, I suppose, which only occurs when type checking __init__.pyi itself.

[hynek]

Maybe I was unclear. What I meant is putting the logic into __init__.py_i_. you can leave everything as it is, just don't start a new file?

[layday]

I created a new file because the MYPY constant is automatically exported because it's not (can't be) underscored and will appear in code completion, etc.

[layday]

We can keep just MYPY in the new stub file and import it from __init__.pyi, but then we'll have the problem I described with defining the protocol:

from ._typing_compat import MYPY



if MYPY:
    class AttrsInstance(Protocol):
        __attrs_attrs__: ClassVar[Any]

else:
    AttrsInstance = Any

# Pyright ignores the else cond



if MYPY:
    class AttrsInstance(Protocol):
        __attrs_attrs__: ClassVar[Any]

else:
    AttrsInstance: Any

# Pyright reports that 'Class declaration "AttrsInstance" is obscured by a declaration of the same name'
# for `AttrsInstance: Any`



class AttrsInstance_(Protocol):
    __attrs_attrs__: ClassVar[Any]

if MYPY:
    AttrsInstance = AttrsInstance_
else:
    AttrsInstance = Any

# No errors in either type checker but mypy displays the original name with the underscore: `type[AttrsInstance_]`
# which is probably no better than having it under a different ns

[hynek]

I can live with MYPY in our stubs if that's all that's stopping you? Sorry on the go and want to unblock you, but can't peruse in detail

[layday]

Okay, there are several issues here.

The first one is not wanting to expose MYPY. Ununderscored assignments in stub files are automatically "exported". In contrast, ununderscored imports are not automatically re-exported. We can't rename MYPY so the only alternative is to tuck it away in a private stub file and import it from there (or not import it at all).

The second issue (the thorniest of them all) is conditionally declaring a type. You can typically only do this using a small set of constants. Thinking about this some more, that the MYPY condition works at all in Pyright is probably a bug, because it should not be possible to redeclare a type alias and have it function as a union, i.e. AttrsInstance_ should not be interpreted as a type alias. I think the current approach is definitely wrong. We could try:

if MYPY:
    # A protocol to be able to statically accept an attrs class.
    class AttrsInstance_(Protocol):
        __attrs_attrs__: ClassVar[Any]
else:
    class AttrsInstance_(Protocol):
        pass

This will produce a localised type error in Pyright about AttrsInstance_ being redecleared. But what does Pyright do after that? Does it create a union? reveal_type(AttrsInstance_) prints out Type of "AttrsInstance_" is "Type[AttrsInstance_]" and __attrs_attrs__ is not accessible on AttrsInstance_, which, ostensibly, is what we want.

The third issue is retaining the type's original qualified name (attr.AttrsInstance). This is essentially only possible if AttrsInstance is defined in attr the way it's done in main. If we want to satisfy #1, then I think the only solution to this is to subclass the protocol from _typing_compat into a new protocol:

class AttrsInstance(AttrsInstance_, Protocol):
    pass

[Tinche]

So, uh. I think this protocol will need to be importable, so people can mix it in. So it'll need to be in a .py file instead.

Does that complicate things?

[layday]

It shouldn't matter.

[hynek]

just a heads up: i've stolen the protection logic because i had to protect the pyright tests against 3.7 anyways. i've credited you, but sorry for the conflicts

[layday]

That's no problem at all :)

---

Eric Traut has commented on the implementation here at microsoft/pyright#3808 (reply in thread).

[layday]

Any thoughts on how to proceed here? I would like to change the chromatic composition of my CI eventually 😛

[hynek]

It looks good enough to my ignorant eyes – @Tinche?

[Tinche]

This looks good to me, but I'd really like to have the protocol importable by normal code. @layday can you refactor this logic out of .pyi files?

[layday]

Do you still want it to be defined in __init__?

[layday]

Bear in mind that Protocol was added in 3.8, so if you want this to be available at runtime, I think we have three choices:

• Drop support for Python <= 3.7
• Add typing-extensions as a required dependency
• Maintain separate runtime and typing implementations of AttrsInstance -- the runtime class would not derive from Protocol

[Tinche]

I tried installing this into our of our services and it seems to work great!

[Tinche]

I guess the current implementation is going with #3, which seems fine to me.

[Tinche]

@layday Add a town crier fragment please and we're good to merge.

[Tinche]

Thanks. @hynek this can go in I think.

[hynek]

Thanks everyone!