[docs] fix pyright issues from adding context type hints #17193
Conversation
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
| @@ -166,8 +167,8 @@ def image_sensor(context: SensorEvaluationContext): | |||
| new_images = [ | |||
| img_filename | |||
| for img_filename in os.listdir(os.getenv("MY_DIRECTORY")) | |||
| if not context.instance.has_dynamic_partition( | |||
| images_partitions_def.name, img_filename | |||
| if not images_partitions_def.has_partition_key( | |||
There was a problem hiding this comment.
@clairelin135 interested in your opinion if this is an ok pattern to suggest. The issue in the old snippet was that images_partitions_def.name is an Optional[str], but context.instance.has_dynamic_partition is expecting a str. Switching to this method bypasses the optionality issue.
There was a problem hiding this comment.
yup, this looks reasonable to me
| previous_model_accuracy = materialization.asset_materialization.metadata[ | ||
| "model_accuracy" | ||
| ] | ||
| previous_model_accuracy = None |
There was a problem hiding this comment.
@odette-elementl interested in your sign-off on these changes. Most of it is type casting so that the comparisons are between float types instead of float and MetadataValue which pyright doesn't think is valid
| @@ -29,7 +29,7 @@ def get_iris_data_for_date(*args, **kwargs): | |||
| }, | |||
| ) | |||
| def iris_data_partitioned(context: AssetExecutionContext) -> pd.DataFrame: | |||
| partition = partition = context.partition_key.keys_by_dimension | |||
| partition = partition = context.partition_key.keys_by_dimension # type: ignore | |||
There was a problem hiding this comment.
i dont have a better idea for this one. We know the partition_key is going to be a MultiPartitionKey, but the context.partition_key property is annotated to return only a str for backcompat reasons (i think). If we do a condition on isinstance(context.partition_key, MultiPartitionKey) then there's the else case that would return None, which would force this asset to be a optional-return asset, which would be too complicated for this example
jamiedemaria
force-pushed
the
jamie/docs-type-hints
branch
from
10ed171 to
9c752b8
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints-pyright
branch
from
adcc725 to
dfac25f
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints
branch
from
9c752b8 to
676eb12
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints-pyright
branch
from
dfac25f to
10c16d9
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints
branch
from
676eb12 to
0ffd8cf
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints-pyright
branch
from
10c16d9 to
0a53756
Compare
|
@yuhan could I get your review on these code snippet changes for the docs? |
jamiedemaria
force-pushed
the
jamie/docs-type-hints
branch
from
0ffd8cf to
0c8b44c
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints-pyright
branch
from
0a53756 to
2d9144e
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints
branch
from
0c8b44c to
a958f6b
Compare
jamiedemaria
force-pushed
the
jamie/docs-type-hints-pyright
branch
from
ae3ec0d to
d2390d6
Compare
Summary & Motivation
Adding type hints to the
contextresulted in several pyright errors. These mostly happened when methods on thecontexthad Optional return types, but we were not handling theNonereturn on the calling side. I did my best to handle these cases in the code snippets to accurately model what users may need to do in their code.How I Tested These Changes