You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The helper functions to load assets in load_assets_from_modules all return Sequence[Union[A, B, C]] (where [A, B, C] stands for AssetsDefinition, SourceAsset, Cacheable...). However, these functions explicitly return a list not a generic Sequence.
For type annotations, concrete functions/classes should specify the actual type of what they return. In this case, these functions all returns lists, NOT a generic Sequence.
The confusion might have arisen from the fact that, for appropriate generality in a library like Dagster, functions and classes should be as generic as possible in what they accept as input but be as specific as possible in what they return as output.
The helper functions in this module all returns lists. So it is safe to change them to their actual return type of list. As lists, they are still Sequence[Union[A, B, C], and will naturally pass the type check for any functions that expects input of a generic Sequence[Union[A, B ,C]. But are also more concretely List[Union[A, B, C] and should be typed as such so that one can actually use the returned objects in their own user code.
Examples of the issues:
If I try to append or manipulate the returns, they have the incorrect type of Sequence.
A sequence does not have .append for instance. It
cannot be concatenated with a list of AssetDefinitions,
etc. If using strict type checking, which we do, I
cannot even easily combine the output of these functions
with assets imported "manually". I would get the error
list[AssetsDefinition] is incompatiable with Sequence[...].
It is quite annoying to have to typing.cast the output
of library code from Dagster into its actual type just to
get my own type checking to work. It is made harder by the
fact that CacheableAssetsDefintion is not exported by default,
so I have to import it, and cast the output to list[AssetsDefinition | SourceAsset | Cacheable...]].
What did you expect to happen?
No response
How to reproduce?
No response
Deployment type
None
Deployment details
No response
Additional information
No response
Message from the maintainers
Impacted by this issue? Give it a 👍! We factor engagement into prioritization.
The text was updated successfully, but these errors were encountered:
Dagster version
1.7.4
What's the issue?
The helper functions to load assets in load_assets_from_modules all return
Sequence[Union[A, B, C]]
(where[A, B, C]
stands forAssetsDefinition
,SourceAsset
,Cacheable...
). However, these functions explicitly return alist
not a genericSequence
.For type annotations, concrete functions/classes should specify the actual type of what they return. In this case, these functions all returns lists, NOT a generic Sequence.
The confusion might have arisen from the fact that, for appropriate generality in a library like Dagster, functions and classes should be as generic as possible in what they accept as input but be as specific as possible in what they return as output.
The helper functions in this module all returns lists. So it is safe to change them to their actual return type of list. As lists, they are still
Sequence[Union[A, B, C]
, and will naturally pass the type check for any functions that expects input of a genericSequence[Union[A, B ,C]
. But are also more concretelyList[Union[A, B, C]
and should be typed as such so that one can actually use the returned objects in their own user code.Examples of the issues:
If I try to append or manipulate the returns, they have the incorrect type of Sequence.
A sequence does not have .append for instance. It
cannot be concatenated with a list of AssetDefinitions,
etc. If using strict type checking, which we do, I
cannot even easily combine the output of these functions
with assets imported "manually". I would get the error
list[AssetsDefinition] is incompatiable with Sequence[...].
It is quite annoying to have to
typing.cast
the outputof library code from Dagster into its actual type just to
get my own type checking to work. It is made harder by the
fact that CacheableAssetsDefintion is not exported by default,
so I have to import it, and cast the output to
list[AssetsDefinition | SourceAsset | Cacheable...]]
.What did you expect to happen?
No response
How to reproduce?
No response
Deployment type
None
Deployment details
No response
Additional information
No response
Message from the maintainers
Impacted by this issue? Give it a 👍! We factor engagement into prioritization.
The text was updated successfully, but these errors were encountered: