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

validation triggered too late #461

Closed
IaroslavR opened this issue Nov 3, 2018 · 4 comments
Closed

validation triggered too late #461

IaroslavR opened this issue Nov 3, 2018 · 4 comments

Comments

@IaroslavR
Copy link

attrs 18.2.0
python 3.6.7
I'm not sure, is it bug or expected behavior, but steps to reproduce:

import attr


def validate_x(_, __, value):
    if value not in ["a", "b", ]:
        raise ValueError(f"Unknown type: {value}")


@attr.s(auto_attribs=True)
class A:
    name: str = attr.ib(validator=validate_x)
    map: dict = {
        "x": 1,
        "y": 2,
    }
    detector: str = attr.ib()

    @detector.default
    def init_detector(self):
        return self.map[self.name]


a = A('c')

Expected(by me):

Traceback (most recent call last):
  File "/home/mirror/tst.py", line 23, in <module>
    a = A('c')
  File "<attrs generated init 090efc2f40404e910a622fb3a7382642c4cc7dba>", line 9, in __init__
  File "/home/mirror/tst.py", line 6, in validate_x
    raise ValueError(f"Unknown type: {value}")
ValueError: Unknown type: c

Real:

Traceback (most recent call last):
  File "/home/mirror/tst.py", line 23, in <module>
    a = A('c')
  File "<attrs generated init b58e3f79027d88b47fd9d82dccbb4d2f365e3754>", line 7, in __init__
  File "/home/mirror/tst.py", line 20, in init_detector
    return self.map[self.name]
KeyError: 'c'
@vlcinsky
Copy link

vlcinsky commented Nov 5, 2018

This is expected behaviour.

As stated at http://www.attrs.org/en/stable/init.html?highlight=order#callables

Since the validators runs after the instance is initialized, you can refer to other attributes while validating:

As defaults are evaluated during initialization, the init_detector runs before the validation can take effect.

@hynek
Copy link
Member

hynek commented Nov 6, 2018

Yes this is indeed on purpose and simply a trade off call we had to make. I believe the way we chose to operate is overall more flexible.

@vlcinsky
Copy link

vlcinsky commented Nov 6, 2018

Documentation would benefit from one clear statement about order of particular function execution. Currently one has to hunt it in multiple places.

Here is my proposal with demonstration about how it really works:

"""Order of applying attribute default, converter and validator

For each attribute one by one: apply default, then converter.

For each attribute: apply validator

For the instance, call `__attrs_post_init__`

"""
from functools import partial

import attr
from structlog import get_logger

log = get_logger()


def str2int(attname, text):
    log.info("converter", attname=attname)
    return text


def def_factory(attname):
    log.info("def_factory", attname=attname)
    return 123


def is_positive(attname, instance, attribute, value):
    log.info("validator", attname=attname)
    if value <= 0:
        raise ValueError("Value is not positive")


@attr.s
class A:
    a: int = attr.ib(
        default=attr.Factory(partial(def_factory, "a")),
        converter=partial(str2int, "a"),
        validator=partial(is_positive, "a"),
    )
    b: int = attr.ib(
        default=attr.Factory(partial(def_factory, "b")),
        converter=partial(str2int, "b"),
        validator=partial(is_positive, "b"),
    )
    da: int = attr.ib(converter=partial(str2int, "da"))
    db: int = attr.ib(converter=partial(str2int, "db"))

    def __attrs_post_init__(self):
        log.info("within __attrs_post_init__")

    @da.default
    def _da_default(self):
        log.info("default", attname="da")
        return 24

    @da.validator
    def _da_validator(self, attribute, value):
        log.info("validator", attname="da")
        if value <= 0:
            raise ValueError("Value is not positive")

    @db.default
    def _db_default(self):
        log.info("default", attname="db")
        return 24

    @db.validator
    def _db_validator(self, attribute, value):
        log.info("validator", attname="db")
        if value <= 0:
            raise ValueError("Value is not positive")


def test_it():
    inst = A()
    print("inst", inst)

Running it (as a pytest driven test case) results in following output:

2018-11-06 15:28.33 def_factory                    attname=a
2018-11-06 15:28.33 converter                      attname=a
2018-11-06 15:28.33 def_factory                    attname=b
2018-11-06 15:28.33 converter                      attname=b
2018-11-06 15:28.33 default                        attname=da
2018-11-06 15:28.33 converter                      attname=da
2018-11-06 15:28.33 default                        attname=db
2018-11-06 15:28.33 converter                      attname=db
2018-11-06 15:28.33 validator                      attname=a
2018-11-06 15:28.33 validator                      attname=b
2018-11-06 15:28.33 validator                      attname=da
2018-11-06 15:28.33 validator                      attname=db
2018-11-06 15:28.33 within __attrs_post_init__
inst A(a=123, b=123, da=24, db=24)

@IaroslavR
Copy link
Author

@vlcinsky Tnx. Cristal clear explanation

hynek added a commit that referenced this issue Jan 22, 2019
@hynek hynek closed this as completed in #488 Feb 9, 2019
hynek added a commit that referenced this issue Feb 9, 2019
* Clarify execution order in init

Fixes #461

* Clarify attributes are processed in the order of declaration

* Simple past is good enough
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

3 participants