From 361eaeca2c212713e1cc58565995957c0d21b3fd Mon Sep 17 00:00:00 2001 From: Deepyaman Datta Date: Fri, 17 Jan 2025 17:19:45 -0700 Subject: [PATCH 1/3] Use `isinstance()` over `hasattr()` for type guard --- .../operators/boolean_operators.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py index d3371d110fa8e..72b9615306644 100644 --- a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py +++ b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py @@ -11,6 +11,9 @@ BuiltinAutomationCondition, ) from dagster._core.definitions.declarative_automation.automation_context import AutomationContext +from dagster._core.definitions.declarative_automation.operators.dep_operators import ( + DepsAutomationCondition, +) from dagster._record import copy, record from dagster._serdes.serdes import whitelist_for_serdes @@ -18,6 +21,18 @@ from dagster._core.definitions.asset_selection import AssetSelection +def _has_allow_ignore(condition: AutomationCondition) -> bool: + return isinstance( + condition, + ( + DepsAutomationCondition, + AndAutomationCondition, + OrAutomationCondition, + NotAutomationCondition, + ), + ) + + @whitelist_for_serdes(storage_name="AndAssetCondition") @record class AndAutomationCondition(BuiltinAutomationCondition[T_EntityKey]): @@ -98,7 +113,7 @@ def allow(self, selection: "AssetSelection") -> "AndAutomationCondition": return copy( self, operands=[ - child.allow(selection) if hasattr(child, "allow") else child + child.allow(selection) if _has_allow_ignore(child) else child for child in self.operands ], ) @@ -118,7 +133,7 @@ def ignore(self, selection: "AssetSelection") -> "AndAutomationCondition": return copy( self, operands=[ - child.ignore(selection) if hasattr(child, "ignore") else child + child.ignore(selection) if _has_allow_ignore(child) else child for child in self.operands ], ) @@ -199,7 +214,7 @@ def allow(self, selection: "AssetSelection") -> "OrAutomationCondition": return copy( self, operands=[ - child.allow(selection) if hasattr(child, "allow") else child + child.allow(selection) if _has_allow_ignore(child) else child for child in self.operands ], ) @@ -219,7 +234,7 @@ def ignore(self, selection: "AssetSelection") -> "OrAutomationCondition": return copy( self, operands=[ - child.ignore(selection) if hasattr(child, "ignore") else child + child.ignore(selection) if _has_allow_ignore(child) else child for child in self.operands ], ) @@ -288,7 +303,7 @@ def allow(self, selection: "AssetSelection") -> "NotAutomationCondition": return copy( self, operand=self.operand.allow(selection) - if hasattr(self.operand, "allow") + if _has_allow_ignore(self.operand) else self.operand, ) @@ -307,6 +322,6 @@ def ignore(self, selection: "AssetSelection") -> "NotAutomationCondition": return copy( self, operand=self.operand.ignore(selection) - if hasattr(self.operand, "ignore") + if _has_allow_ignore(self.operand) else self.operand, ) From 87d99e2c75bcc4324fc1af3ff5f05eaf0c3e043a Mon Sep 17 00:00:00 2001 From: Deepyaman Datta Date: Fri, 17 Jan 2025 20:44:07 -0700 Subject: [PATCH 2/3] Try to more explicitly narrow the type with TypeIs --- .../operators/boolean_operators.py | 27 +++++++++++-------- python_modules/dagster/setup.py | 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py index 72b9615306644..1b15440f2e00c 100644 --- a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py +++ b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py @@ -1,6 +1,8 @@ import asyncio from collections.abc import Sequence -from typing import TYPE_CHECKING, Union +from typing import TYPE_CHECKING, TypeVar, Union, get_args + +from typing_extensions import TypeIs import dagster._check as check from dagster._annotations import public @@ -21,16 +23,19 @@ from dagster._core.definitions.asset_selection import AssetSelection -def _has_allow_ignore(condition: AutomationCondition) -> bool: - return isinstance( - condition, - ( - DepsAutomationCondition, - AndAutomationCondition, - OrAutomationCondition, - NotAutomationCondition, - ), - ) +T_HasAllowIgnore = TypeVar( + "T_HasAllowIgnore", + bound=Union[ + DepsAutomationCondition, + "AndAutomationCondition", + "OrAutomationCondition", + "NotAutomationCondition", + ], +) + + +def _has_allow_ignore(condition: AutomationCondition) -> TypeIs[T_HasAllowIgnore]: + return isinstance(condition, get_args(T_HasAllowIgnore.__bound__)) @whitelist_for_serdes(storage_name="AndAssetCondition") diff --git a/python_modules/dagster/setup.py b/python_modules/dagster/setup.py index 98f9cefa2d2fe..e68a16937697b 100644 --- a/python_modules/dagster/setup.py +++ b/python_modules/dagster/setup.py @@ -95,7 +95,7 @@ def get_version() -> str: "tabulate", "tomli<3", "tqdm<5", - "typing_extensions>=4.4.0,<5", + "typing_extensions>=4.10.0,<5", 'tzdata; platform_system=="Windows"', "structlog", "sqlalchemy>=1.0,<3", From 552b6c1a33a8d10b2a99cbbdf4816c652db53f24 Mon Sep 17 00:00:00 2001 From: Deepyaman Datta Date: Fri, 17 Jan 2025 21:33:02 -0700 Subject: [PATCH 3/3] Don't use bound type alias; use type union instead --- .../operators/boolean_operators.py | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py index 1b15440f2e00c..53325ea89d847 100644 --- a/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py +++ b/python_modules/dagster/dagster/_core/definitions/declarative_automation/operators/boolean_operators.py @@ -1,6 +1,6 @@ import asyncio from collections.abc import Sequence -from typing import TYPE_CHECKING, TypeVar, Union, get_args +from typing import TYPE_CHECKING, Union from typing_extensions import TypeIs @@ -23,19 +23,31 @@ from dagster._core.definitions.asset_selection import AssetSelection -T_HasAllowIgnore = TypeVar( - "T_HasAllowIgnore", - bound=Union[ +def _has_allow_ignore( + condition: AutomationCondition, +) -> TypeIs[ + Union[ DepsAutomationCondition, "AndAutomationCondition", "OrAutomationCondition", "NotAutomationCondition", - ], -) - - -def _has_allow_ignore(condition: AutomationCondition) -> TypeIs[T_HasAllowIgnore]: - return isinstance(condition, get_args(T_HasAllowIgnore.__bound__)) + ] +]: + from dagster._core.definitions.declarative_automation.operators.boolean_operators import ( + AndAutomationCondition, + NotAutomationCondition, + OrAutomationCondition, + ) + + return isinstance( + condition, + ( + DepsAutomationCondition, + AndAutomationCondition, + OrAutomationCondition, + NotAutomationCondition, + ), + ) @whitelist_for_serdes(storage_name="AndAssetCondition")