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

cattrs.structure is failing detailed_validation=True behavior with validations #440

Open
joshua-alday opened this issue Nov 3, 2023 · 6 comments

Comments

@joshua-alday
Copy link

  • cattrs version: 23.1.2
  • Python version: 3.10
  • Operating System: windows 10
  • IDE: Pycharm 2023.1 Professional Edition

Description

cattrs.structure is seemingly failing to properly aggregate validation failures, even when explicitly setting detailed_validation=True on new converter.

What I Did

This will result in ['invalid value @ $'] which is unexpected.

import cattrs
from attrs import define
from attrs import field, validators

@define
class TestModel:
    test_data_1: float = field(validator=[validators.instance_of(float), validators.lt(100)])
    test_data_2: str = field(validator=validators.max_len(3))

    @test_data_1.validator
    def _check_scale(self, attribute, value):
        if int(log10(value)) + 1 > 1:
            raise ValueError("value significant figures is greater than 1")

data = {"test_data_1": 110, "test_data_2": "abcdefj"}

try:
    print(cattrs.structure(data, TestModel))
except cattrs.BaseValidationError as e:
    print(cattrs.transform_error(e))

If I print out the exceptions from e: (ValueError("'test_data_1' must be < 100: 110.0"),)

Unless I'm missing something very critical, this seems like a bug.

@Tinche
Copy link
Member

Tinche commented Nov 3, 2023

You're mixing attrs validators and cattrs here. It's not a mistake or anything, but you should be aware attrs validators don't aggregate all errors; they short-circuit on the first one.

You can see this if you try instantiating your model directly:

>>> TestModel(110, "abcdefj")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<attrs generated init a13.TestModel>", line 6, in __init__
  File "/Users/tintvrtkovic/pg/cattrs/.venv/lib/python3.12/site-packages/attr/_make.py", line 2927, in __call__
    v(inst, attr, value)
  File "/Users/tintvrtkovic/pg/cattrs/.venv/lib/python3.12/site-packages/attr/validators.py", line 100, in __call__
    raise TypeError(
TypeError: ("'test_data_1' must be <class 'float'> (got 110 that is a <class 'int'>).", Attribute(name='test_data_1', default=NOTHING, validator=_AndValidator(_validators=(<instance_of validator for type <class 'float'>>, <Validator for x < 100>, <function TestModel._check_scale at 0x10c7704a0>)), repr=True, eq=True, eq_key=None, order=True, order_key=None, hash=None, init=True, metadata=mappingproxy({}), type=<class 'float'>, converter=None, kw_only=False, inherited=False, on_setattr=None, alias='test_data_1'), <class 'float'>, 110)

So there's nothing we can do directly here. cattrs doesn't disable attrs validators for various reasons.

There is an effort underway to add a better validation framework to cattrs though, which would probably solve your issue. Right now cattrs will validate the shape of your data (if a field is annotated as an int, it'll be an int or bust) but there's no way to validate on the actual values. So that's coming.

@joshua-alday
Copy link
Author

@Tinche
Okay, this actually makes sense now. I didn't think about attrs being the source of the issue. Thank you for informing me of this. Do we have an idea of when this update will be made? I'm rolling attrs/cattrs into my API project for request validation models. I don't have to heavily use validators at the moment, but it would be good to fully implement them later on.

@Tinche
Copy link
Member

Tinche commented Nov 6, 2023

It's coming in 24.1, I think in 3-6 months. Sorry :/

@joshua-alday
Copy link
Author

No need to apologize. I can work around the issue as proper a engineer should haha. This will give me a chance to get better familiarized with exception attributes. With a little bit of intuition I can have something working soon.

@Tinche
Copy link
Member

Tinche commented Nov 6, 2023

I was OK with keeping this open and marked as an enhancement request until we implement this validation, up to you.

@joshua-alday
Copy link
Author

Sure, yeah. I just didn't think of it as an immediate issue. But if it helps to mark as a request then let's do that.

@joshua-alday joshua-alday reopened this Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants