jscheffl commented on code in PR #36652:
URL: https://github.com/apache/airflow/pull/36652#discussion_r1444087360
##########
airflow/decorators/base.py:
##########
@@ -351,7 +351,7 @@ def fake():
except TypeError: # Can't evaluate return type.
return False
ttype = getattr(return_type, "__origin__", return_type)
- return ttype is dict or ttype is Dict
+ return issubclass(ttype, Dict)
Review Comment:
I am not 100% a type checking expert. A dictionary is also just marked by it
is "mappable" and "Mapping" is the abstract base class for this. I agree that
the previous check was not good but I don't feel like `issubclass()` is also
covering all use cases. How about this?
```suggestion
return isinstance(ttype, Mapping)
```
See also:
https://docs.python.org/3.8/library/collections.abc.html#collections-abstract-base-classes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
