Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util.ImmutableValidatedObject: Support Optional[ImmutableValidatedObject] #1482

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rehno-lindeque
Copy link

Similar to 88494fb this adds support for Optional[ImmutableValidatedObject]

Usage

  • Support for optional sub-modules with required sub-options.
  • Simplify auto-generating plugin code from API definitions that follow this pattern.

E.g. this is not currently possible:

{
  misc = mkOption {
    default = null;
    type = types.nullOr (types.submodule miscOptions);
  };
}

Considerations

Supporting additional generic types

It's trivial to add support forMapping[Any, ImmutableValidatedObject] as well, but I didn't want to add new code that isn't actively used in a plugin.

Satisfying mypy

Mypy needs to be upgraded to remove type: ignore[attr-defined] after typing.get_args and typing.get_origin.

Never-the-less the tests pass just fine with this version of python:

$ python --version
Python 3.8.7

@rehno-lindeque rehno-lindeque force-pushed the immutable-validated-object-optional branch from 46ea7c9 to fdfbd13 Compare November 1, 2021 22:26
Comment on lines +170 to +177
# Support Sequence[ImmutableValidatedObject]
if isinstance(value, Sequence) and issubclass(tuple, type_origin):
value = tuple(_transform_value(v, type_args[0]) for v in value)

# Support Optional[ImmutableValidatedObject]
if type_origin is Union:
if value is not None:
value = _transform_value(value, type_args[0])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered that the logic for this is slightly off, so hold off reviewing for now please.

@rehno-lindeque rehno-lindeque marked this pull request as draft November 5, 2021 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant