Mypy raises error when using `dagster.Config` in Dagster 1.5.5

[christeefy]

Dagster version

dagster, version 1.5.5

What's the issue?

Been eagerly awaiting to use Pydantic 2+ with Dagster 1.5.5.

However, my project's CI was failing with mypy with the following error wherever dagster.Config was used:

Metaclass conflict: the metaclass of a derived class must be a (non-strict) subcla ss of the metaclasses of all its bases

I'm aware that your team uses Pyright, so I used it and had no metaclass errors in my codebase or in the example above.

This results from a fundamental difference between mypy and pyright in understanding the type of ModelMetaclass (see Additional Information section below).

While it can be argued that the issue is at the type-checker level, I wonder if there's something that can be done within this codebase to unblock mypy users, who may not be able to move to pyright (immediately).

What did you expect to happen?

mypy should not complain when running the example below.

How to reproduce?

# my_file.py
from dagster import Config

class MyConfig(Config):
    ...

>>> mypy my_file.py
my_file.py:4: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the 
metaclasses of all its bases  [misc]                                                                                                   
Found 1 error in 1 file (checked 1 source file)       

Deployment type

None

Deployment details

No response

Additional information

Fundamentally, the issue stems from a behaviour difference between mypy and pyright with this import statement:

# snippet.py

try:
    # Pydantic 2.x
    from pydantic.main import ModelMetaclass
except ImportError:
    # Pydantic 1.x
    from pydantic._internal._model_construction import ModelMetaclass  # type: ignore

reveal_type(ModelMetaclass)

When running the snippet above:

>>> mypy snippet.py
note: Revealed type is "Any"  

>>> pyright snippet.py
information: Type of "ModelMetaclass" is "Any | type[ModelMetaclass]"                                                                  

I've tried to fix it similar to this suggestion on Pydantic but to no avail.

Curious to hear thoughts on what the path forward may look like for mypy users of Dagster. Also happy to contribute with guidance 😊

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.

[christeefy]

I've tried switching the ordering of the import statements

try:
    # Pydantic 2.x
    from pydantic._internal._model_construction import ModelMetaclass  # type: ignore
except ImportError:
    # Pydantic 1.x
    from pydantic.main import ModelMetaclass

mypy no longer complains on Pydantic 2x but on Pydantic 1x 😅

[christeefy]

Side note: I think the # Pydantic 2.x and # Pydantic 1.x comments in the current codebase should be flipped.

[judahrand]

Is this something which is likely to be fixed? @gibsondan

[C0DK]

Would be lovely for this to be addressed - or has it been (silently) fixed already?

[Daniel-Vetter-Coverwhale]

This still does not appear to be fixed as of dagster 1.6.1. It's hard to deal with as well, because we'd need to ignore all of the problems that mypy lumps into [misc], which could cover up other issues.

[alangenfeld]

It's hard to deal with as well, because we'd need to ignore all of the problems that mypy lumps into [misc], which could cover up other issues

Just to clarify, are you concerned that there are other misc errors that would attribute to the specific line when you are doing something like:

from dagster import Config

class MyConfig(Config):  # type: ignore[misc]
    ...

https://mypy.readthedocs.io/en/stable/common_issues.html#spurious-errors-and-locally-silencing-the-checker

While not ideal I would expect other misc errors that would attribute to that line to be pretty unlikely and for these type ignores to be a reasonable path forward in the interim.

I have not been able to figure out a solution to the core problem.

[judahrand]

While not ideal I would expect other misc errors that would attribute to that line to be pretty unlikely and for these type ignores to be a reasonable path forward in the interim.

Our Dagster project is now littered with these ignores 😔 It rather pollutes the readability of the codebase and I really hope the Dagster team are able to dedicate some time to it.

[Daniel-Vetter-Coverwhale]

Agreed with the above about readability.

I'm mostly just glad to hear you guys are looking into it, even if it hasn't been solved yet. Do you have thoughts on flipping the imports or maybe picking them based on pydantic version - https://docs.pydantic.dev/latest/api/version/? It looks like switching the ordering worked to satisfy mypy for @christeefy, though when I switch the ordering of that import I wind up seeing this other error again - https://github.com/dagster-io/dagster/issues/16723

My current workaround that I just thought of today is to run mypy . | rg -v "Metaclass conflict" rather than have the type ignores or have misc in my list of disabled error codes for mypy. It seems to work alright, though it doesn't feel the best lol

[alangenfeld]

Unfortunately we find ourselves in the situation outlined here https://github.com/pydantic/pydantic/issues/6022#issuecomment-1671643844

Switching the order of the imports in the try except just changes who gets the error from pydnatic 2 users to pydantic 1 users.

Switching to an if pydantic_version block instead of try catch nets the same results.

Switching from importing ModelMetaclass to using type(BaseModel) resulted in errors across both pydantic versions, same with all the other type coercion I attempted.

Unless someone is able to find a clever solution it appears the only choice is to flip who gets the errors at some point. Looking at https://www.pepy.tech/projects/pydantic?versions=2.*&versions=1.* it appears v2 downloads are just starting to outpace v1 so maybe that time is soon.

[dwallace0723]

Just giving a 👍 here that our Dagster projects are also now littered with these errors. Hopefully we get a solution soon!