You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "josh-fell (via GitHub)" <gi...@apache.org> on 2023/02/09 04:21:16 UTC

[GitHub] [airflow] josh-fell opened a new pull request, #29436: Handle `multiple_outputs` inferral with PEP 563 + TYPE_CHECKING

josh-fell opened a new pull request, #29436:
URL: https://github.com/apache/airflow/pull/29436

   Closes: #29435
   
   In the context of the TaskFlow API, when using PEP 563 along with `typing.TYPE_CHECKING` DAG import errors are thrown due to the type-hint evaluation for `multiple_outputs`. It seems as though the `typing_extensions.get_type_hints()` call has an issue with typing imports behind `TYPE_CHECKING`.
   
   In the even of a `NameError`, fallback to using `func.__annotations__` to retrieve the return type annotation for a TaskFlow function to infer `multiple_outputs`.
   
   TODO:
   - [ ] Add tests


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on a diff in pull request #29436: Handle `multiple_outputs` inferral with TYPE_CHECKING

Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #29436:
URL: https://github.com/apache/airflow/pull/29436#discussion_r1103975725


##########
airflow/decorators/base.py:
##########
@@ -299,6 +299,12 @@ def _infer_multiple_outputs(self):
             return_type = typing_extensions.get_type_hints(self.function).get("return", Any)
         except TypeError:  # Can't evaluate return type.
             return False
+        except NameError:  # A typing import is not defined; probably behind TYPE_CHECKING.
+            return_type = self.function.__annotations__.get("return", Any)
+            # If using PEP 563, the return annotation value is stringified. Need to transform to a type to
+            # properly check against dict type.
+            if isinstance(return_type, str):
+                return_type = eval(return_type)

Review Comment:
   I think this could still raise NameError in some cases. Maybe we should return False as a last resort instead of failing?



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell closed pull request #29436: Handle `multiple_outputs` inferral with TYPE_CHECKING

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell closed pull request #29436: Handle `multiple_outputs` inferral with TYPE_CHECKING
URL: https://github.com/apache/airflow/pull/29436


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell commented on a diff in pull request #29436: Handle `multiple_outputs` inferral with TYPE_CHECKING

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29436:
URL: https://github.com/apache/airflow/pull/29436#discussion_r1104527770


##########
airflow/decorators/base.py:
##########
@@ -299,6 +299,12 @@ def _infer_multiple_outputs(self):
             return_type = typing_extensions.get_type_hints(self.function).get("return", Any)
         except TypeError:  # Can't evaluate return type.
             return False
+        except NameError:  # A typing import is not defined; probably behind TYPE_CHECKING.
+            return_type = self.function.__annotations__.get("return", Any)
+            # If using PEP 563, the return annotation value is stringified. Need to transform to a type to
+            # properly check against dict type.
+            if isinstance(return_type, str):
+                return_type = eval(return_type)

Review Comment:
   #29445 was used instead of this PR to address https://github.com/apache/airflow/issues/29435. I'll close this PR. Not sure why this PR wasn't closed automatically since it's tied to the `Closes` keyword in the other PR. I hope this didn't waste too much of your time!



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell commented on pull request #29436: Handle `multiple_outputs` inferral with TYPE_CHECKING

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on PR #29436:
URL: https://github.com/apache/airflow/pull/29436#issuecomment-1428010565

   Deferring to #29445. Closing.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org