Skip to content

Check pyerr in postcohtable

Timothy Davies requested to merge tdavies__check_pyerr_in_postcohtable into spiir-O4-EW-development

from_buffer should always either set a pyerr & return null, or not set a pyerr & not return null.

Previously, PyList_SetItem could set an error, and we'd return a valid pyobject, so python wouldn't throw the error until it tried the next inbuilt function. That messed up the stack trace.

I Think the same is true of some of our gets/sets, but I don't wanna mess with those at this stage.

We've tested the following, similar branch here: !147 (comment 644543) (https://git.ligo.org/lscsoft/spiir/-/commits/tdavies__test_pyerr/)

If we don't have debug output like in that branch, we won't know the exact line that went wrong. We could leave them, overwrite/append to the PyErr, or just add debug and rerun after an error occurs.

There's some cases where we've done this:

        Complex8TimeSeriesWrapper *wrapped_snr_series =
          (Complex8TimeSeriesWrapper *)PyType_GenericNew(
            (PyTypeObject *)wrapped_snr_series_class, NULL, NULL);
        if (!wrapped_snr_series) {
            PyErr_SetString(
              PyExc_ValueError,
              "Failed to create wrapped snr series: constructor error");
            return NULL;
        }

But maybe it's better not to swallow some error set by PyType_GenericNew? (I'm not sure if it can even return null in the first place...)

Edited by Timothy Davies

Merge request reports