Skip to content

Commit

Permalink
[pythonic config] properly propagate default values set at the top le…
Browse files Browse the repository at this point in the history
…vel for nested config classes (#27206)

## Summary

Ensures that default values on Pythonic Config get properly propagated
to inner Config classes, e.g.

```python
class ANestedOpConfig(Config):
    an_int: int = 1
    a_bool: bool = True

class AnOpConfig(Config):
    a_string: str = "foo"
    a_nested: ANestedOpConfig = ANestedOpConfig(an_int=5)
```

An alternative impl which uses some utils in a stacked PR can be found
at #27209.

## Test Plan

New unit test; todo add a few more
  • Loading branch information
benpankow authored and prha committed Jan 29, 2025
1 parent d3418ef commit abff5e4
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ def infer_schema_from_config_class(
model_cls: type["Config"],
description: Optional[str] = None,
fields_to_omit: Optional[set[str]] = None,
default: Optional[Any] = None,
) -> DagsterField:
from dagster._config.pythonic_config.config import Config
from dagster._config.pythonic_config.resource import (
Expand Down Expand Up @@ -433,9 +434,11 @@ def infer_schema_from_config_class(
" definition. 'dagster.Field' should only be used in legacy Dagster config"
" schemas. Did you mean to use 'pydantic.Field' instead?"
)

field_default = default.get(resolved_field_name) if isinstance(default, dict) else None
try:
fields[resolved_field_name] = _convert_pydantic_field(pydantic_field_info)
fields[resolved_field_name] = _convert_pydantic_field(
pydantic_field_info, default=field_default
)
except DagsterInvalidConfigDefinitionError as e:
raise DagsterInvalidPythonicConfigDefinitionError(
config_class=model_cls,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ def _curry_config_schema(schema_field: Field, data: Any) -> DefinitionConfigSche


def _convert_pydantic_field(
pydantic_field: ModelFieldCompat, model_cls: Optional[type] = None
pydantic_field: ModelFieldCompat,
model_cls: Optional[type] = None,
default: Optional[Mapping[str, Any]] = None,
) -> Field:
"""Transforms a Pydantic field into a corresponding Dagster config field.
Expand All @@ -104,9 +106,11 @@ def _convert_pydantic_field(

field_type = pydantic_field.annotation
if safe_is_subclass(field_type, Config):
default = None
if pydantic_field.default and isinstance(pydantic_field.default, Config):
default = pydantic_field.default._get_non_default_public_field_values() # noqa: SLF001
inferred_field = infer_schema_from_config_class(
field_type,
description=pydantic_field.description,
field_type, description=pydantic_field.description, default=default
)
return inferred_field
else:
Expand All @@ -116,9 +120,13 @@ def _convert_pydantic_field(
config_type = _config_type_for_type_on_pydantic_field(field_type)

default_to_pass = (
pydantic_field.default
if pydantic_field.default is not PydanticUndefined
else FIELD_NO_DEFAULT_PROVIDED
default
if default
else (
pydantic_field.default
if pydantic_field.default is not PydanticUndefined
else FIELD_NO_DEFAULT_PROVIDED
)
)
if isinstance(default_to_pass, Enum):
default_to_pass = default_to_pass.name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,51 @@ def a_job():
a_job.execute_in_process({"ops": {"a_struct_config_op": {"config": {"a_string": "foo"}}}})

assert executed["yes"]


def test_default_values_nested_override():
class InnermostConfig(Config):
a_float: float = 1.0
another_float: float = 2.0

class ANestedOpConfig(Config):
an_int: int = 1
a_bool: bool = True
inner_config: InnermostConfig = InnermostConfig(another_float=1.0)

class AnOpConfig(Config):
a_string: str = "foo"
a_nested: ANestedOpConfig = ANestedOpConfig(an_int=5)

executed = {}

@op
def a_struct_config_op(config: AnOpConfig):
assert config.a_string == "foo"
assert config.a_nested.an_int == 5
assert config.a_nested.a_bool is True
assert config.a_nested.inner_config.a_float == 3.0
assert config.a_nested.inner_config.another_float == 1.0
executed["yes"] = True

from dagster._core.definitions.decorators.op_decorator import DecoratedOpFunction

assert DecoratedOpFunction(a_struct_config_op).has_config_arg()

@job
def a_job():
a_struct_config_op()

assert a_job

a_job.execute_in_process(
{
"ops": {
"a_struct_config_op": {
"config": {"a_string": "foo", "a_nested": {"inner_config": {"a_float": 3.0}}}
}
}
}
)

assert executed["yes"]

0 comments on commit abff5e4

Please sign in to comment.