BUG: Propagate Python exceptions from c_is_list_like (#33721)
- [x] closes #33721
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

May also be an issue that `is_array` is defined in `pandas/_libs/tslibs/util.pxd` but this seems to try and access from `pandas/_libs/util.pxd` > May also be an issue that is_array is defined in pandas/_libs/tslibs/util.pxd but this seems to try and access from pandas/_libs/util.pxd

Out of skepticism I tried changing the cimport and it didnt change the segfault. > I still don't think this is the right solution. As is this function would never return -1 so it's a little confusing to declare that a return value of -1 should propogate errors.

The fact that the function will never return -1 is exactly why it is used to signal exceptions. Here are the relevant Cython docs: 

https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#error-return-values

This pattern is also used in other places in `lib.pyx`:

https://github.com/pandas-dev/pandas/blob/master/pandas/_libs/lib.pyx#L1096 > Do you know within this function where the segfault actually happens? I think the issue is probably with util.is_array if anything

In this particular case, a segfault happens because a `RecursionError` is thrown from within the function (because the stack happens to be exhausted there), but is ignored. The whatsnew can be removed, then LGTM Misunderstanding on my end - thanks for the link. 

Lgtm

Sent from my iPhone

> On Apr 23, 2020, at 10:10 AM, Jeff Reback <notifications@github.com> wrote:
> 
> ﻿
> @jreback requested changes on this pull request.
> 
> In pandas/_libs/lib.pxd:
> 
> > @@ -1 +1 @@
> -cdef bint c_is_list_like(object, bint)
> +cdef bint c_is_list_like(object, bint) except -1
> -1 is appropriate here as it signals cython that there maybe is an exception to check.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.
 Thanks -- will do. Any suggestions for where to put that test? > Thanks -- will do. Any suggestions for where to put that test?

pandas/tests/dtypes/test_inference.py I'm now having second thoughts about the robustness of this test: it assumes that the `RecursionError` will be raised from within `c_is_list_like`, but it could as well be raised from anywhere else within `is_list_like()`. Granted, for now, `is_list_like()` just calls `c_is_list_like`, but that could conceivably change in the future.

Any thoughts here? > Any thoughts here?

what if got rid of c_is_list_like and just made is_list_like `cpdef`? That would solve this problem. But it looks like `c_is_list_like` is used elsewhere within the Cython (e.g., https://github.com/pandas-dev/pandas/blob/master/pandas/_libs/reshape.pyx#L124).

Not sure whether the overhead that will be introduced by calling a `cpdef` method instead is something to worry about. I think that having a fast `cdef` method is valuable here. @shwina can you post the test that you have for now? Will be easier for review and filing comments lgtm can u merge master and resolve conflicts and ping in green Thanks @jreback - done. thanks @shwina 