@seberg I realized when I sat back down to work on this, I wasn't exactly clear of how to setup a test for the fail case. I'm stumbling a little bit on how to create an object that will fail when copy.deepcopy is called specifically from within PyObject_CallFunctionObjArgs. I originally tried making a dummy class within the test but I was only able to make the high level .__deepcopy__ behavior fail, which isn't exactly what we're looking for.

def test__deepcopy__catches_failure(self):
    class MyArray(np.ndarray):
         def __deepcopy__(self, *args, **kwargs):
             raise Exception

    a = MyArray([1, 2, 3], dtype='O')
    a.__deepcopy__({})  # Raises an `Exception` defined above, not one from within `_deepcopy_call` C code

I also tried mocking out deepcopy itself but also didn't seem to get all the way there. It wasn't clear to me what to assert for in the case when the underlying deepcopy fails.

def test__deepcopy__catches_failure(self):
    a = np.array([1, 2, 3], dtype='O')

    with patch.object(copy, 'deepcopy', side_effect=None):
        b = a.__deepcopy__({})
      
        # What would a valid assertion be here?
        assert (b == None).all()

Do you have any suggestions or examples from other places in the code of how to do this?

@jcusick13 oh, you have to put hte bad object into the array:

    class MyObj(np.ndarray):
         def __deepcopy__(self, *args, **kwargs):
             raise Exception

    arr = np.array([MyObj()])
    arr.__deepcopy__({})  # or copy.deepcopy(arr), I guess

Thanks @seberg. I added the suggested changes but am still a little bit confused. From looking at the docs for PyObject_CallFunctionObjArgs, I'm expecting errors to be caught within that function and then returned as NULL

Return the result of the call on success, or raise an exception and return NULL on failure.

though it seems like the Exception raised from the bad object just bubbles all the way back up to the surface rather than being caught within the error handling of _deepcopy_call:

/* call deepcopy on this argument */
res = PyObject_CallFunctionObjArgs(deepcopy, itemp, visit, NULL);
Py_DECREF(itemp);
if (res == NULL) {
    return -1;
}

How exceptions work (typically), is that you return NULL indicating that an error occurred (somewhere). Python then sets the global error state that you could check for example with PyErr_Occured(). So NULL result means "error", and the error itself is stored elsewhere. Was that your question?

Ah thanks for the explanation @seberg. I think my final point of confusion is if we then expect a failure within this function

res = PyObject_CallFunctionObjArgs(deepcopy, itemp, visit, NULL);

to raise a Python Exception or if it should return NULL, which would then be handled more gracefully but the error handling within _deepcopy_call?

Mainly, it's not clear to me what to check during the test for the fail case. What should I be looking to create an assert for in the end of the test for this behavior?

Thanks also for that last error test! Hope to see you around :). Lets put this in, it looks nice to me.