Offer a simpler way to resolve metaclass conflicts

[robsdedude]

Feature

Consider the following minimal example

assert type(tuple) is type

class MyMeta(type): pass

class MyTuple(tuple, metaclass=MyMeta): pass

As of 0.990, mypy complains about this

error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases  [misc]

Same if I use

class MyMeta(tuple.__class__): pass

or

class MyMeta(type(tuple)): pass

instead

Pitch

I propose 2 alternatives to help users deal with this

• Teach mypy that tuple.__class__ is type and that this is fine (preferred)
• Tell users to use # typing: ignore to resolve such cases.
  Technically, this is already possible today, but I suggest 2 improvements:
  • https://mypy.readthedocs.io/en/stable/metaclasses.html#gotchas-and-limitations-of-metaclass-support only lists what mypy can't handle. It does not talk about how to work around it => add a note telling users to # typing: ignore[misc] it. I'd like to mention that this feels pretty broad of an exception and might hide other real problems. Since this is an area where mypy might be overly restrictive and a workaround might be necessary, it should maybe be in a less broad category.
  • This is the more important point to me: this comment needs to be added to every subclass of MyTuple which can be frustratingly tedious. Maybe there should be away to tell mypy "ok, you might not understand this, but trust me that this class MyTuple has a valid metaclass, assume so for all child classes".

[robsdedude]

As I played further with it, I found that annotating class MyMeta(type(tuple)): pass with # typing: ignore[misc] allows me to not have to annotate every child class. That's something, I guess. But at least the docs should tell people about this workaround. Ideally, as said, it shouldn't even be necessary.

[sobolevn]

Right now tuple is defined as class tuple(Sequence[_T_co], Generic[_T_co]): (and it is not just tuple, other types are affected as well)

So, it has ABCMeta metaclass according to the typeshed 🤔
I don't think we can remove it.

Your ideas seem reasonable. Do you want to work on it? :)

[AlexWaygood]

Right now tuple is defined as class tuple(Sequence[_T_co], Generic[_T_co]): (and it is not just tuple, other types are affected as well)

So, it has ABCMeta metaclass according to the typeshed 🤔 I don't think we can remove it.

This problem was previously discussed in python/typeshed#1595

[robsdedude]

Do you want to work on it? :)

I'm happy to contribute. a) I haven't contributed to mypy before, so I wouldn't exactly know where to start, guidance welcome b) Can you elaborate on "Your ideas seem reasonable"? I'm not sure which of the two approaches I mentioned you'd like to see me implement. If the first one: I wouldn't know where to start looking. Judging by what AlexWaygood wrote, this seems to not be solvable in mypy, but typshed which again would need extended typing features in Python itself... so that seems a little unfeasible to me?

I'm happy to jump into a chat somewhere to talk about the details, if you want to help me getting started.

[sobolevn]

only lists what mypy can't handle. It does not talk about how to work around it => add a note telling users to # typing: ignore[misc] it. I'd like to mention that this feels pretty broad of an exception and might hide other real problems. Since this is an area where mypy might be overly restrictive and a workaround might be necessary, it should maybe be in a less broad category.

There are two cool ideas in this:

1. We can totally list this as a known problem in the docs
2. We can use a separate error code for metaclass issues

@robsdedude you can schedule a meeting if you want: https://calendly.com/sobolevn/ Don't worry that it says 2hours (we can do much faster). I will guide you through the contribution process :)

Or drop me an email: mail @ sobolevn.me if that's easier for you.

[sobolevn]

@robsdedude meeting notes:

1. Error codes: https://github.com/python/mypy/blob/master/mypy/errorcodes.py
2. Error messages: https://github.com/python/mypy/blob/master/mypy/message_registry.py
3. Metaclass check:

   mypy/mypy/checker.py

   Lines 2496 to 2523 in dbcbb3f

   	def check_metaclass_compatibility(self, typ: TypeInfo) -> None:
   	"""Ensures that metaclasses of all parent types are compatible."""
   	if (
   	typ.is_metaclass()
   	or typ.is_protocol
   	or typ.is_named_tuple
   	or typ.is_enum
   	or typ.typeddict_type is not None
   	):
   	return # Reasonable exceptions from this check
   	metaclasses = [
   	entry.metaclass_type
   	for entry in typ.mro[1:-1]
   	if entry.metaclass_type
   	and not is_named_instance(entry.metaclass_type, "builtins.type")
   	]
   	if not metaclasses:
   	return
   	if typ.metaclass_type is not None and all(
   	is_subtype(typ.metaclass_type, meta) for meta in metaclasses
   	):
   	return
   	self.fail(
   	"Metaclass conflict: the metaclass of a derived class must be "
   	"a (non-strict) subclass of the metaclasses of all its bases",
   	typ,
   	)

4. Tests:

   mypy/test-data/unit/check-classes.test

   Lines 6821 to 6852 in dbcbb3f

   	[case testMetaclassConflict]
   	class MyMeta1(type): ...
   	class MyMeta2(type): ...
   	class MyMeta3(type): ...
   	class A(metaclass=MyMeta1): ...
   	class B(metaclass=MyMeta2): ...
   	class C(metaclass=type): ...
   	class A1(A): ...
   	class E: ...
   	class CorrectMeta(MyMeta1, MyMeta2): ...
   	class CorrectSubclass1(A1, B, E, metaclass=CorrectMeta): ...
   	class CorrectSubclass2(A, B, E, metaclass=CorrectMeta): ...
   	class CorrectSubclass3(B, A, metaclass=CorrectMeta): ...
   	class ChildOfCorrectSubclass1(CorrectSubclass1): ...
   	class CorrectWithType1(C, A1): ...
   	class CorrectWithType2(B, C): ...
   	class Conflict1(A1, B, E): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	class Conflict2(A, B): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	class Conflict3(B, A): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	class ChildOfConflict1(Conflict3): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	class ChildOfConflict2(Conflict3, metaclass=CorrectMeta): ...
   	class ConflictingMeta(MyMeta1, MyMeta3): ...
   	class Conflict4(A1, B, E, metaclass=ConflictingMeta): ... # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	class ChildOfCorrectButWrongMeta(CorrectSubclass1, metaclass=ConflictingMeta): # E: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
   	...

   To run them: pytest -k TEST_NAME

[robsdedude]

Digging further into it and working on improving the error experience (including the message) as discussed in our meeting, I'm looking at testMetaclassSubclassSelf and in particular ChildOfConflict1. My opinion on this is that, while mypy is (most of the time, anyway) correct in assuming that this class has a metaclass conflict, I don't think the error helps the user at all, because this is not where the error originates (which would be the class definition of Conflict3). I think, this behavior of mypy just a) pollutes the output, making it harder for the user to find the place in the code they need to fix and b) should mypy be incorrect about some assumptions (like with some built-in types), it's not enough to just mute the single place of failure (Conflict3) but every child class of it as well, which can be very tedious.

What do you think about only emitting a metaclass conflict at the place it's introduced at?

This poses some technical difficulty as it's unclear, which metaclass mypy should assume for a class it considered having a metaclass conflict. But I think this can be resolved by traversing the mro and accepting the first explicitly declared metaclass as truth while falling back to type if there is none (which I believe can't actually happen).

Thoughts on this proposal?

[erictraut]

@sobolevn, you said "So, it has ABCMeta metaclass according to the typeshed". Where is ABCMeta introduced? Does mypy assume that all Protocol classes implicitly have ABCMeta as a metaclass? Or perhaps it assumes this for protocol classes that have one or more @abstractmethod? Pyright doesn't make this assumption, so it doesn't report a metaclass conflict in the code sample at the top of this thread. I mention this because it could be a simple solution if mypy were not to assume that ABCMeta is the metaclass for protocols.

[robsdedude]

Another thing to discuss: changing the error code is a breaking change. For users that have silenced mypy complaining about metaclass conflicts (# type: ignore[misc]) this change would make their typecheck start to fail. So I don't know how you handle these kind of things.

Moreover, I noticed that changing the error code didn't make any tests fail. So it appears there are no tests for those, are there?

[sobolevn]

In my opinion changing codes from misc to some_exact_code is a good thing.

Because it adds more context to it. And changing failing places is rather easy for end users.
But, I am open for other opinions.

[sobolevn]

@erictraut yes, this is something I will investigate. Thanks!

[robsdedude]

For the record: I found that mypy 0.990 happily accepts

class M1(type): pass
class M2(type): pass
class Mx(M1, M2): pass

class A1(metaclass=M1): pass
class A2(A1): pass

class B1(metaclass=M2): pass

class C1(metaclass=Mx): pass

class D(A2, B1, C1): pass

While the interpreter is, rightfully so, not amused.

Traceback (most recent call last):
  File "foo.py", line 12, in <module>
    class D(A2, B1, C1): pass
TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

I'll see if I can also address this bug as it is very much related.

Note for myself: class D(A2, C1, B1): pass however, is fine.

[robsdedude]

Reading Python's documentation it seems mypy should actually be right and Python should accept class D(A2, B1, C1): pass. So I belief this is either a bug in the interpreter or the docs aren't quite accurate.

Can someone shed some light on which one is the case?

[erictraut]

I think the interpreter is correct here. Mypy should flag D as a metaclass conflict. (FWIW, pyright reports an error here.)

As you noted, if you change the order of the base classes for D and move C1 before B1, the problem is eliminated.

[robsdedude]

Sorry for the notification, but I've been waiting patiently for my PRs to get reviewed without luck. Could somebody please have a look before they go even more stale and have even more merge conflicts?

[mukundjalan]

@JukkaL reaching out to you because you are the top contributor for this repository. Could you please help get this PR reviewed and merged as a lot of us are facing issues due to this.
Thanks & apologies for bothering you.