Attrs: Add a convenience flag for upcalling on Exception subclasses

Created on 11 Apr 2018  Â·  7Comments  Â·  Source: python-attrs/attrs

Currently, these tests both fail on 2.7 and only the latter fails on 3.6.

@attr.s
class E1(Exception):
    x = attr.ib()
    y = attr.ib()


def test_exception_init_called():
    e = E1(1, 2)
    assert e.args == (e.x, e.y) == (1, 2)


def test_exception_init_called_even_with_kw():
    e = E1(y=3, x=4)
    assert e.args == (e.x, e.y) == (4, 3)

If attrs is overriding __init__ then this makes sense; args is filled in by __init__ in 2.x and __new__ in 3.x, but only pulling from the *args and never **kwargs. I discussed this a bit with @markrwilliams but I'm not sure what the best way to handle an attrs-ified exception would be. Maybe it's okay to consider args vestigal.

Mostly I'm raising this on the issue tracker because there are some references to using attrs for exceptions, but only indirectly. It'd be nice to have some documentation on a suggested approach either way.

Feature

Most helpful comment

So, what we want here is basically:

  1. @attr.s(str=True)
  2. All fields additionally put into self.args?
  3. Exceptions usually can’t be compared – so maybe also cmp=False?

I’d be sympathetic to add something like @attr.exc since I use it quite often myself.

All 7 comments

Is test_exception_init_called_even_with_kw a valid test?

The args attribute on Exception is documented as "The tuple of arguments given to the exception constructor."

And Exception itself doesn't accept kwargs. So I think the behavior in the case of kwargs to init with respect to what goes into the args attribute is… undefined. So I'd have no expectation of that test passing on exception classes generically.

I included it because of the usual attrs isomorphism:

class E1(Exception):
    def __init__(self, x, y):
        super(E1, self).__init__(x, y)
        self.x = x
        self.y = y

It might not abide strictly by the documentation of Exception that you'd end up with e.args == (e.x, e.y) but I wasn't sure if a case could be made since this is how one would write the class without attrs.

I guess what I'm saying is that it's not clear to me that when one calls the super init, that including the kwargs is a common practice.

That said, I find the args attribute's design to be severely lacking in clarity, basically a "tuple of whatever rando stuff the caller threw in here", so I'm rather sympathetic to the argument that it is vestigal.

The only reason to care about it as far as I'm aware is that it effects str(), and that's true even when using attrs, which leaves something to be desired:

>>> str(C(1,2))
'C(x=1, y=2)'
>>> str(C(x=1,y=2))
'C(x=1, y=2)'
>>> str(E1(1,2))
'(1, 2)'
>>> str(E1(x=1,y=2))
''

Here E1 is defined as in your example, and C is the same as E1, but without inheriting from Exception.

After running into #217 I came up with an @attrs_exception decorator that passes both of your tests on Python 2.7 (and probably 3.6):

import inspect

import attr


def attrs_exception(maybe_cls=None, *args, **kwargs):
    r"""
    A class decorator to eliminate boilerplate when creating Exceptions using
    :func:`attr.s`.

    Adds ``__str__`` (because Exceptions don't use ``__repr__``) and an
    ``__attrs_post_init__`` that calls ``Exception.__init__`` with the class's
    :func:`attr.ib`\ s. Without this, instances cannot be deepcopied on Python 2.

    ref: https://github.com/python-attrs/attrs/issues/217

    :param args: additional positional arguments to be passed to :func:`attr.s`
    :param kwargs: additional keyword arguments to be passed to :func:`attr.s`
    """
    def wrap(cls):
        def _attrs_post_init__(obj):
            Exception.__init__(obj, *attr.astuple(obj))

        existing_post_init = getattr(cls, '__attrs_post_init__', None)
        if (
            existing_post_init
            and (
                inspect.getsource(existing_post_init)
                != inspect.getsource(_attrs_post_init__)
            )
        ):
            raise TypeError(
                "'%s' already has an '__attrs_post_init__' method" % (cls,),
            )
        if not issubclass(cls, Exception):
            raise TypeError(
                "'%s' is not a subclass of Exception! Use 'attr.s' instead" % (cls,),
            )
        cls.__attrs_post_init__ = _attrs_post_init__

        kwargs['str'] = True
        kwargs['slots'] = True
        return attr.attributes(cls, *args, **kwargs)

    # maybe_cls depends on the usage of the decorator. It's a class if it's
    # used as @attrs_exception but None if it's used as @attrs_exception()
    if maybe_cls is None:
        return wrap
    else:
        return wrap(maybe_cls)

If anyone is interested, I can probably put that (and its accompanying tests) on PyPI.

So, what we want here is basically:

  1. @attr.s(str=True)
  2. All fields additionally put into self.args?
  3. Exceptions usually can’t be compared – so maybe also cmp=False?

I’d be sympathetic to add something like @attr.exc since I use it quite often myself.

Just hit this myself, blindly assuming Exceptions would work - @attr.exc (@attr.err?) would be very handy.

I took a stab at this in #500.

Please provide feedback so we can close this gaping hole.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hynek picture hynek  Â·  12Comments

Bananaman picture Bananaman  Â·  7Comments

indigoviolet picture indigoviolet  Â·  8Comments

ojii picture ojii  Â·  16Comments

2mol picture 2mol  Â·  5Comments