On 2021-09-30 2:04 p.m., Chris Angelico wrote:
On Fri, Oct 1, 2021 at 1:10 AM Soni L. <fakedme+py@gmail.com> wrote:
On 2021-09-30 11:23 a.m., Chris Angelico wrote:
For example this: (real code)
def get_property_values(self, prop): try: factory = self.get_supported_properties()[prop] except KeyError as exc: raise PropertyError from exc iterator = factory(self._obj) try: first = next(iterator) except StopIteration: return (x for x in ()) except abdl.exceptions.ValidationError as exc: raise LookupError from exc except LookupError as exc: raise RuntimeError from exc # don't accidentally swallow bugs in the iterator return itertools.chain([first], iterator)
There's a lot of exception transformation happening here, and I don't understand, without context, the full purpose of it all. But the fact that you're raising LookupError directly seems a tad odd (it's a parent class for IndexError and KeyError), and it seems like a custom exception type would solve all of this. I *think* what's happening here is that you transform ValidationError into LookupError, but only if it happens on the very first yielded result?? Then how about:
class PropertyNotFoundError(LookupError): pass
def get_property_values(self, prop): try: factory = self.get_supported_properties()[prop] except KeyError as exc: raise PropertyError from exc iterator = factory(self._obj) try: yield next(iterator) except StopIteration: pass except abdl.exceptions.ValidationError as exc: raise PropertyNotFoundError from exc yield from iterator
If the factory raises some other LookupError, then that's not a PropertyNotFoundError. And if it *does* raise PropertyNotFoundError, then that is clearly deliberate, and it's a signal that the property is, in fact, not found.
Your code, as currently written (and including the rewrite), is *still* capable of leaking a faulty exception - just as long as the factory returns one good value first. So I'm not sure what you actually gain by this transformation.
Okay let us stop you right there. This generator is not a function.
Generators don't do anything until iterated.
There's a HUGE semantic difference there. You're making assumptions that you could easily invalidate yourself if you paid attention to the original code. Don't do that.
try: iterator = foo.get_property_values(prop) except PropertyError, etc: pass # handle it for thing in iterator: pass # exceptions in the iteration actually get propagated because they mean something went wrong and the program should terminate. in the real code, this includes spurious ValidationError etc, because those are *never* supposed to happen.
Congratulations, you showed me some extremely complicated code, and then got miffed when I messed up something in trying to simplify it. Well done, you successfully tripped me up in my attempt to make code better.
Fair enough, we went too far there, sorry.
Can we stick to the point, please? You still haven't shown why (a) a context manager, nor (b) a custom exception, cannot solve this problem far better than this extremely magical syntax.
This syntax is far from magical. We're not sure how the context manager would work here, but, a custom exception doesn't work, because you still have the... whatever you'd call this issue: https://mail.python.org/pipermail/python-ideas/2017-June/046109.html (one of Steven's links also linked to this and it's fairly relevant.) The imports issue sadly wouldn't be solved with this feature, but attributes would: def __getattr__(self, name) with AttributeError: stuff and things if the thing: return some thing raise AttributeError This also applies anywhere else you'd have something wrapping another thing and they happen to use the same exceptions in slightly distinct ways. Yes, it's most noticeable with StopIteration (which got tweaked with the change to generators), imports (because the modules you depend on generally also depend on other modules), and attributes (because you might do a foo.typo in your __getattr__ and it's not particularly uncommon to dynamically probe attributes). But it does happen in other cases too. (KeyError is an interesting one, especially when making a wrapper over a dict/subclassing dict/etc.) In one of the examples, we showed this function: https://github.com/ganarchy/GAnarchy/blob/993a8ca85db1564e64550276d61d972342... In reality, we really should also be catching any PropertyError in the unpacking, just in case someone happened to implement get_property_values incorrectly and it waited until iteration to raise, because that's a case where a bug would be swallowed. And the only reason we didn't bother to guard against ValueError during iteration is simply this: property kinds have arity, some are 1-ary and some are n-ary. it's a programming bug to call get_property_value on an n-ary property, so we decided to assume nobody's gonna actually catch those ValueError. This is, in practice, an extremely dangerous assumption, especially in the wider ecosystem, because, as it's a documented part of the API, ppl do assume the implementation is strictly correct about it. But it would also be wrong to not make it a documented part of the API.
ChrisA _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/YY6T5J... Code of Conduct: http://python.org/psf/codeofconduct/