result icon indicating copy to clipboard operation
result copied to clipboard

feat: add overload to unwrap_or_raise to raise the exception itself

Open mokeyish opened this issue 1 year ago • 5 comments

add overload to unwrap_or_raise to raise the exception itself

   n2 = Err(ValueError('nay'))
   with pytest.raises(ValueError) as exc_info:
        n2.unwrap_or_raise()

mokeyish avatar Jul 23 '24 08:07 mokeyish

lacking a clear motivation for this change (hint hint), i'm not convinced how this is useful looking at the diff only. is the intention to make try/except blocks simpler?

some context:

.unwrap_or_raise() is intended to raise an instance of the specified exception class. this pull request changes that to make that optional, and raise whatever the Err value was, but only if it the error value is an exception.

for Err types that hold an exception instance, plain .unwrap() already attaches that exception instance to the raised UnwrapException, via its .__cause__ attribute:

>>> import result
>>> err = result.Err(ValueError("oops"))
>>> try:
...     err.unwrap()
... except result.UnwrapError as exc:
...     print(repr(exc))
...     print(repr(exc.__cause__))
... 
UnwrapError("Called `Result.unwrap()` on an `Err` value: ValueError('oops')")
ValueError('oops')

wbolster avatar Jul 25 '24 09:07 wbolster

I just want the ValueError, not the UnwrapError. due to external code doesn't know about UnwrapError.

Without this PR, I can only use err.unwrap_or_raise(lambda e: e), it's a bit troublesome.

mokeyish avatar Jul 25 '24 09:07 mokeyish

@wbolster what about a new unwrap_or_raise_self method that does what @mokeyish is trying to do here. But leaving the existing unwrap_or_raise unchanged.

francium avatar Aug 08 '24 03:08 francium

this PR is strictly an API addition that is backwards compatible. before, the argument was required:

def unwrap_or_raise(self, e: Type[TBE]) -> NoReturn:

after, the argument is optional:

    def unwrap_or_raise(self, e: Optional[Type[TBE]] = None) -> NoReturn:

the behaviour when passing an exception type (which was required before this PR) stays the same.

so i don't think this needs a new method at all. in fact, with the new info and reasoning, i think this change is fine as-is. wdyt @francium?

wbolster avatar Aug 08 '24 13:08 wbolster

fwiw, though the mentioned work-around may work, it's not type safe:

err.unwrap_or_raise(lambda e: e)

… b/c that lambda is not an exception type.

wbolster avatar Aug 08 '24 13:08 wbolster