dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
11.06k stars 1.38k forks source link

Support type hints for arbitrary length tuples #10345

Open treilly1 opened 1 year ago

treilly1 commented 1 year ago

What's the use case?

According to PEP 484 type hints

Arbitrary-length homogeneous tuples can be expressed using one type and ellipsis, for example Tuple[int, ...]. (The ... here are part of the syntax, a literal ellipsis.)

However, when using that syntax dagster throws error

../.venv/lib/python3.9/site-packages/dagster/_core/definitions/input.py:336: in _checked_inferred_type
    resolved_type = resolve_dagster_type(inferred.annotation)
../.venv/lib/python3.9/site-packages/dagster/_core/types/dagster_type.py:888: in resolve_dagster_type
    dagster_type = transform_typing_type(dagster_type)
../.venv/lib/python3.9/site-packages/dagster/_core/types/transform_typing.py:40: in transform_typing_type
    return create_typed_tuple(*transformed_types)
../.venv/lib/python3.9/site-packages/dagster/_core/types/python_tuple.py:88: in create_typed_tuple
    dagster_types = list(map(resolve_dagster_type, dagster_type_args))
../.venv/lib/python3.9/site-packages/dagster/_core/types/dagster_type.py:928: in resolve_dagster_type
    raise DagsterInvalidDefinitionError(
E   dagster._core.errors.DagsterInvalidDefinitionError: Invalid type: dagster_type must be an instance of DagsterType or a Python type: got Ellipsis.

Ideas of implementation

No response

Additional information

using dagster 1.0.10

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.

smackesey commented 1 year ago

Thanks for pointing this out, we need to either support this or handle it more gracefully.

danielgafni commented 1 year ago

Seems like right now even specific-length tuples are not supported in the Pythonic config system?

from dagster import asset, materialize, Config

def test_tuple_config():
    class MyConfig(Config):
        tuple_field: Tuple[int, int] = (1, 2)

    @asset
    def my_asset(config: MyConfig) -> int:
        return config.tuple_field[0] + config.tuple_field[1]

    materialize([my_asset])

raises

Traceback (most recent call last):
  File "/home/dan/.local/share/JetBrains/Toolbox/apps/PyCharm-P/ch-0/222.4459.20/plugins/python/helpers/pycharm/_jb_unittest_runner.py", line 35, in <module>
    sys.exit(main(argv=args, module=None, testRunner=unittestpy.TeamcityTestRunner, buffer=not JB_DISABLE_BUFFERING))
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/main.py", line 100, in __init__
    self.parseArgs(argv)
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/main.py", line 147, in parseArgs
    self.createTests()
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/main.py", line 158, in createTests
    self.test = self.testLoader.loadTestsFromNames(self.testNames,
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/loader.py", line 220, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/loader.py", line 220, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/dan/.pyenv/versions/3.10.10/lib/python3.10/unittest/loader.py", line 205, in loadTestsFromName
    test = obj()
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/tests/test_definitions.py", line 23, in test_tuple_config
    def my_asset(config: MyConfig) -> int:
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_core/definitions/decorators/asset_decorator.py", line 208, in asset
    return create_asset()(compute_fn)
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_core/definitions/decorators/asset_decorator.py", line 346, in __call__
    op = _Op(
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_core/definitions/decorators/op_decorator.py", line 105, in __call__
    self.config_schema = infer_schema_from_config_annotation(
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_config/pythonic_config/__init__.py", line 1673, in infer_schema_from_config_annotation
    return infer_schema_from_config_class(model_cls)
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_config/pythonic_config/__init__.py", line 1715, in infer_schema_from_config_class
    fields[pydantic_field.alias] = _convert_pydantic_field(
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_config/pythonic_config/__init__.py", line 1487, in _convert_pydantic_field
    inner_field = _get_inner_field_if_exists(pydantic_field.shape, pydantic_field)
  File "/home/dan/Projects/sanas/science/mlops/feature-extractors/teacher-whisper/.venv/lib/python3.10/site-packages/dagster/_config/pythonic_config/__init__.py", line 1448, in _get_inner_field_if_exists
    raise NotImplementedError(f"Pydantic shape type {shape_type} not supported.")
NotImplementedError: Pydantic shape type 5 not supported.

Process finished with exit code 1

Empty suite
smackesey commented 1 year ago

cc @benpankow

ian-scale commented 8 months ago

still seeing issues with tuples not being supported: Trying to define this list of tuples results in:

class EvalConfig(Config):
    eval_benchmarks: List[Tuple[str, int]] = [] # list of tuples of (benchmark, eval_k)

dagster._core.errors.DagsterInvalidDefinitionError: You have passed in typing.Tuple[str, int] to the config system. Types from the typing module in python are not allowed in the config system. You must use types defined by dagster or primitives

for context, this is running a locally deployed dagster deployment on my k8s cluster. @smackesey any ideas on what might be causing this issue?

marioGab commented 6 months ago

@smackesey & @benpankow will this be resolved?