[arrow-pyarrow]: restore nicer pyarrow-arrow error message - #9639
Conversation
alamb
changed the title
Tpt
left a comment
Tpt
left a comment
There was a problem hiding this comment.
Thank you! Some minor improvement idea.
If you want I can prepare a new MR that build a similar nice error for each extraction of the crate while propagating the original error.
| ) -> PyResult<(Bound<'py, PyCapsule>, Bound<'py, PyCapsule>)> { | ||
| let tuple = value.call_method0("__arrow_c_array__")?; | ||
|
|
||
| if !tuple.is_instance_of::<PyTuple>() { |
There was a problem hiding this comment.
if I am not mistaken .extract() will fail if tuple is not a tuple, returning the exact same error. I think we can drop this case.
| )); | ||
| } | ||
|
|
||
| tuple.extract().map_err(|_| { |
There was a problem hiding this comment.
tip if you want to propagate the original error as a "cause" to get a sightly nicer backtrace:
| tuple.extract().map_err(|_| { | |
| tuple.extract().map_err(|e| { | |
| let err = PyTypeError::new_err( | |
| "Expected __arrow_c_array__ to return a tuple of (schema, array) capsules.", | |
| ); | |
| err.set_cause(value.py(), Some(e)); | |
| err |
That would be great -- thank you 🙏 |
|
Closing this one in favor of the work from @Tpt |
|
Actually, since this one is approved, I'll merge it in and then @Tpt can you prepare a follow on PR to improve the messages further? |
# Which issue does this PR close? - Follow on to apache#9594 # Rationale for this change @kylebarron says apache#9594 (comment): > fwiw previously there was a nice user-facing error here, while now the error generated from extract will be much more obtuse. Ideally this exception will never be raised except if the producer doesn't follow the spec correctly. # What changes are included in this PR? Restore the nice error # Are these changes tested? yes, added a test # Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. If there are any breaking changes to public APIs, please call them out. -->
Remove validate_pycapsule: pointer_checked already raises an exception if the capsule type is wrong, and we wrap it into a nicer error Also relies on a trait to associate types with their capsules names Because we wrap the exception (equivalent of `raise X from Y`), the original exception is still there Follow up of #9639 #9594
3 participants
Which issue does this PR close?
Rationale for this change
@kylebarron says #9594 (comment):
What changes are included in this PR?
Restore the nice error
Are these changes tested?
yes, added a test
Are there any user-facing changes?