Skip to content

Conversation

@binaryDiv
Copy link
Contributor

Currently, for implementing a context-sensitive __post_validate__() method in dataclasses, it is required to accept arbitrary keyword arguments (**kwargs). If the __post_validate__() method does not accept those, NO context arguments will be passed to it at all.

This can cause confusion and very annoying debugging, and also there's not really a reason to it other than a slightly simpler implementation.

This PR changes this: __post_validate__() can now be defined with specific keyword-arguments (i.e. only those that your post validation method actually needs) without accepting **kwargs. The DataclassValidator will make sure to only pass parameters to the method that it accepts.

It is also allowed to define them as positional arguments (as long as they are not positional-only), but this is disencouraged and will raise a warning. Please always use keyword-only arguments (i.e. def __post_validate__(self, *, some_param=None) instead of def __post_validate__(self, some_param=None)).

@binaryDiv binaryDiv added the enhancement Improvements to existing features or smaller new features label May 22, 2023
@binaryDiv binaryDiv self-assigned this May 22, 2023
@ninanomenon
Copy link

I understand what you are doing in the code. But I can't assess what the impact is on the rest of the code. That's why I'm having a bit of a hard time reviewing this.

@binaryDiv
Copy link
Contributor Author

binaryDiv commented May 23, 2023

@L1F

I understand what you are doing in the code. But I can't assess what the impact is on the rest of the code. That's why I'm having a bit of a hard time reviewing this.

The rest of the code shouldn't be affected by this change. This has only a effect on validataclasses that have defined a __post_validate__() method (which probably nobody except me really uses anyway). And it basically just makes it easier to use and to avoid mistakes.

Before this change, this code would have a hard-to-find bug:

@validataclass
class ExampleDataclass:
    # [insert validators]

    def __post_validate__(self, *, some_context_arg: Optional[str] = None):
        # [do something with some_context_arg]

validator = DataclassValidator(ExampleDataclass)
result = validator.validate(some_input_data, some_context_arg='example')

What is expected to happen: The validator calls __post_validate__(some_context_arg='example') on the dataclass instance.
What really happens: The validator calls __post_validate__() but without any arguments, because the method doesn't accept **kwargs.

I wrote a warning about this in the documentation, and then made this exact mistake when writing code and was really confused why it didn't work 🙃

This PR just changes how the __post_validate__ method is called depending on how it's defined, so the code above would work now.

@binaryDiv binaryDiv merged commit 23eb3de into main May 24, 2023
@binaryDiv binaryDiv deleted the dataclass-post-validate-without-varkwargs branch May 24, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvements to existing features or smaller new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants