Skip to content

Cannot use TypedDict object when defining params #29358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2 tasks done
jcobb-healx opened this issue Feb 3, 2023 · 6 comments · Fixed by #29782
Closed
2 tasks done

Cannot use TypedDict object when defining params #29358

jcobb-healx opened this issue Feb 3, 2023 · 6 comments · Fixed by #29782
Assignees
Labels
affected_version:2.5 Issues Reported for 2.5 area:core good first issue kind:bug This is a clearly a bug

Comments

@jcobb-healx
Copy link
Contributor

Apache Airflow version

2.5.1

What happened

Context: I am attempting to use TypedDict objects to maintain the keys used in DAG params in a single place, and check for key names across multiple DAGs that use the params.
This raises an error with mypy as params expects an Optional[Dict]. Due to the invariance of Dict, this does not accept TypedDict objects.

What happened: I passed a TypedDict to the params arg of DAG and got a TypeError.

What you think should happen instead

TypedDict objects should be accepted by DAG, which should accept Optional[Mapping[str, Any]].

Unless I'm mistaken, params are converted to a ParamsDict class and therefore the appropriate type hint is a generic Mapping type.

How to reproduce

Steps to reproduce

from typing import TypedDict

from airflow import DAG
from airflow.models import Param

class ParamsTypedDict(TypedDict):
    str_param: Param

params: ParamsTypedDict = {
    "str_param": Param("", type="str")
}

with DAG(
    dag_id="mypy-error-dag",
    # The line below raises a mypy error
    # Argument "params" to "DAG" has incompatible type "ParamsTypedDict"; expected "Optional[Dict[Any, Any]]"  [arg-type]
    params=params,  
) as dag:
    pass

Operating System

Amazon Linux

Versions of Apache Airflow Providers

No response

Deployment

Docker-Compose

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@jcobb-healx jcobb-healx added area:core kind:bug This is a clearly a bug labels Feb 3, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 3, 2023

Thanks for opening your first issue here! Be sure to follow the issue template!

@jcobb-healx
Copy link
Contributor Author

Relevant mypy discussion on passing TypedDict as args annotated with Dict: python/mypy#4976

@potiuk
Copy link
Member

potiuk commented Feb 3, 2023

Feel free to work on it.

@eladkal eladkal added the affected_version:2.5 Issues Reported for 2.5 label Feb 4, 2023
@pierrejeambrun
Copy link
Member

pierrejeambrun commented Feb 25, 2023

@jcobb-healx Are you still working on this ? Don't hesitate to reach out if you need some help :)

@jcobb-healx
Copy link
Contributor Author

@pierrejeambrun Sorry, I hadn't gotten around to looking at this. Spent a bit of time on this today, and it looks like converting the type hint to Mapping isn't right, as the params are merged with the default params in an update call, which isn't consistent with TypedDict behaviour.
Given the earlier mypy discussion, it probably makes sense to use MutableMapping to allow generic inputs, and let users perform casts, i.e. not accept TypedDict objects.

@jcobb-healx
Copy link
Contributor Author

jcobb-healx commented Feb 27, 2023

@pierrejeambrun I've made the changes to use MutableMapping so that passing in a ParamsDict object works as expected. I've not changed anything that would allow my original case to work, so I've made no mention of TypedDict in the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected_version:2.5 Issues Reported for 2.5 area:core good first issue kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants