Skip to content

Add a convenience flag for upcalling on Exception subclasses #368

Closed

Description

@habnabit

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.

Activity

  1. wsanchez commented on Apr 11, 2018

    @wsanchez

    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.

  2. habnabit commented on Apr 11, 2018

    @habnabit
    Author

    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.

  3. wsanchez commented on Apr 11, 2018

    @wsanchez

    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.

  4. rouge8 commented on May 2, 2018

    @rouge8
    Contributor

    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.

  5. hynek commented on Jun 16, 2018

    @hynek
    Member

    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.

  6. added a commit that references this issue on Jun 25, 2018
  7. glyph commented on Oct 15, 2018

    @glyph
    Contributor

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

  8. added this to the import attrs milestone on Feb 9, 2019
  9. added 4 commits that reference this issue on Feb 10, 2019
    4c7e20c
    a111d8d
    a9e552f
    2c02443
  10. hynek commented on Feb 10, 2019

    @hynek
    Member

    I took a stab at this in #500.

    Please provide feedback so we can close this gaping hole.

  11. added 2 commits that reference this issue on Feb 24, 2019
    5250799
    078fcaf
  12. added a commit that references this issue on Feb 25, 2019
    25a98cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions