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

Add an 'attr' attribute to AttributeError #62356

Closed
brettcannon opened this issue Jun 7, 2013 · 16 comments
Closed

Add an 'attr' attribute to AttributeError #62356

brettcannon opened this issue Jun 7, 2013 · 16 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@brettcannon
Copy link
Member

BPO 18156
Nosy @brettcannon, @vstinner, @6502, @benjaminp, @ezio-melotti, @merwok, @alex, @flying-sheep, @serhiy-storchaka, @zhangyangyu
Files
  • attributeerror-attr.patch
  • client.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2013-06-07.15:31:50.189>
    labels = ['interpreter-core', 'type-feature']
    title = "Add an 'attr' attribute to AttributeError"
    updated_at = <Date 2017-08-18.16:16:17.429>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2017-08-18.16:16:17.429>
    actor = 'brett.cannon'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2013-06-07.15:31:50.189>
    creator = 'brett.cannon'
    dependencies = []
    files = ['30829', '40757']
    hgrepos = ['203']
    issue_num = 18156
    keywords = ['patch']
    message_count = 15.0
    messages = ['190754', '190783', '190787', '190788', '190789', '192466', '192468', '192470', '192555', '229928', '230501', '230504', '261307', '261355', '261394']
    nosy_count = 11.0
    nosy_names = ['brett.cannon', 'vstinner', 'ag6502', 'benjamin.peterson', 'ezio.melotti', 'eric.araujo', 'alex', 'flying sheep', 'serhiy.storchaka', 'kermit666', 'xiang.zhang']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'test needed'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18156'
    versions = ['Python 3.4']

    @brettcannon
    Copy link
    Member Author

    Much like ImportError now has 'name' and 'path', AttributeError should get an 'attr' attribute that can only be set through a keyword argument or after creating an instance. That would make the common try/except AttributeError uses much more robust by not accidentally swallowing an AttributeError that has nothing to do with the attribute in question::

    try:
    cls.meth()
    except AttributeError as exc:
    if exc.attr != 'meth':
    raise

    @brettcannon brettcannon added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Jun 7, 2013
    @alex
    Copy link
    Member

    alex commented Jun 8, 2013

    +1 on this, but it's worth noting that that fix is not 100% correct (though it's obviously better than most existing equivilants), it's potentially wrong with custom __getattr__, __getattribute__, descriptors.

    @benjaminp
    Copy link
    Contributor

    Such custom implementations should be updated to support this wonderful new attr. :)

    @brettcannon
    Copy link
    Member Author

    What Benjamin said.

    Adding something like this is mostly about a nicer constructor (AttributeError(attr='meth')) and automatically creating the message for the exception (although that would require another argument like 'object' or something to be able to do e.g. "'dict' object has no attribute 'asdfdsff'" or "type object 'dict' has no attribute 'asdfdsf'").

    @brettcannon
    Copy link
    Member Author

    And standardizing on an attribute name, of course. =)

    @kermit666
    Copy link
    Mannequin

    kermit666 mannequin commented Jul 6, 2013

    I've been working on this at the EuroPython sprint today and it seems the change requires editing >20 files that call PyExc_AttributeError. This means it would be quite a big and dangerous change, so for now I just attach the optional argument addition - without it actually being used by the rest of the codebase.

    What is your oppinion on it?

    @6502
    Copy link
    Mannequin

    6502 mannequin commented Jul 6, 2013

    Even porting to the new wonderful 'attr' field is not going to make the code correct... (the exception could be bubbling up from code down ten frames about a different unrelated attribute that happens to have the same name in a different object). BTW cpython has a lot of those "except AttributeError" fragments coded in C with PyErr_ExceptionMatches.

    @brettcannon
    Copy link
    Member Author

    Dražen: didn't do a deep review, but a cursory look suggests the patch is fine.

    As for having to change a ton of files to start using the attribute, that's part of the effect of changing something as basic as an exception. If you would rather break it up into separate patches that's fine. But I would caution you from doing more than this patch as I have gotten some push back from other core devs on my proposed new attributes on exceptions so I don't want you spending your time on something that might get sunk (although if you are enjoying it then go for it since if it gets accepted this work will be needed).

    Also please sign the contributor agreement form (http://python.org/psf/contrib/contrib-form/) so that we can actually use your code.

    Andrea: While the attribute might coincidentally name an attribute that is the same as some other attribute which is not the actual trigger, the traceback on the exception provides the proper context to know what attribute in what code did the actual triggering. The point is that the exception can be considered better than nothing and is still an improvement over not having the information at all.

    @kermit666
    Copy link
    Mannequin

    kermit666 mannequin commented Jul 7, 2013

    OK, thanks for the feedback. I signed the CLA.

    I'll then wait with the remaining work, until a final decision has been made. We have a rough idea of how it could be implemented if it comes to this - adding a wrapper function in Python/errors.c:

        PyErr_SetAttributeError(PyObject *attr, const char *format, ...)

    that would replace all the PyErr_SetObject, PyErr_SetString and PyErr_Format calls (in around 50 files), create the kwargs object, format the message (if provided) and call PyErr_SetObject or PyErr_SetFormat.

    I put the last patch as a commit in the attr bookmark on BitBucket (took me quite some time to figure out that's the alternative to git branches), so that subsequent changes go more easily.

    @serhiy-storchaka
    Copy link
    Member

    In bpo-22716 proposed to add a reference to the object missing an attribute.

    @ezio-melotti
    Copy link
    Member

    I closed bpo-22716 in favor of this, since I think both should be tackled together.

    @flying-sheep
    Copy link
    Mannequin

    flying-sheep mannequin commented Nov 2, 2014

    yeah, exactly: my idea was to add a reference to the original object (AttributeError.target). together with this bug, that would be the AttributeError part of PEP-473, which i really like!

    @zhangyangyu
    Copy link
    Member

    I'd like to ping this channel.

    I post a question on https://groups.google.com/forum/#!topic/comp.lang.python/y8yDAAJJ9Sc to ask if it is possible to directly get the attribute from AttributeError, which leads me here.

    @vstinner
    Copy link
    Member

    vstinner commented Mar 8, 2016

    try:
    cls.meth()
    except AttributeError as exc:
    if exc.attr != 'meth':
    raise

    What if cls.__getattr__('meth') calls a completly unrelated function which also raises AttributeError(attr='meth') but on a different object? I would also expect an "obj" attribute, a reference to the object which doesn't have attribute.

    But AttributeError can be raised manually without setting attr and/or obj attribute. So the best test would look to something like:

    if exc.obj is cls and exc.attr is not None and exc.attr != 'meth':
        raise

    The test is much more complex than expected :-/ Maybe it's simpler to split the code to first get the bounded method?

    try:
    meth = cls.meth
    except AttributeError:
    ...
    meth()

    Or check first if cls has the attribute 'meth' with hasattr(cls, 'meth')?

    --

    About attr/obj attribute not set, an alternative is to add a new BetterAttributeError(obj, attr) exception which requires obj and attr to be set, it inherits from AttributeError.

    class BetterAttributeError(AttributeError):
       def __init__(self, obj, attr):
           super().__init__('%r has no attribute %r' % (obj, attr)
           self.obj = obj
           self.attr = attr

    It would allow a smoother transition from "legacy" AttributeError to the new BetterAttributeError.

    The major issue with keeping a strong reference to the object is that Python 3 introduced Exception.__traceback__ which can create a reference cycle if an exception is stored in a local variable somewhere in the traceback. It's a major pain point in asyncio. In asyncio, the problem is more likely since exceptions are stored in Future objects to be used later.

    @zhangyangyu
    Copy link
    Member

    The concerns mentioned by haypo seems to have existed in PEP-473.

    @brettcannon brettcannon removed the easy label Aug 18, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @hauntsaninja
    Copy link
    Contributor

    This has existed since Python 3.10 ("name" attribute)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants