From 063a92409f11d14a1d024fd1d620b796b3e012b4 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 24 May 2023 09:36:36 -0400 Subject: [PATCH] Inline Cython exception handler (#13411) I originally placed the exception handler into a separate C++ header file that could be included by the Cython header because I figured that reading C++ inlined in Cython would be more confusing to devs. Unfortunately, the current approach complicates the build system due to the need to ensure that the directory containing the C++ header is always in the include path, which becomes problematic depending on where the files including the exception handler are (anywhere outside of `_lib` becomes problematic). Inlining is the simplest solution to this problem. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Ashwin Srinath (https://github.com/shwina) - Bradley Dice (https://github.com/bdice) URL: https://github.com/rapidsai/cudf/pull/13411 --- python/cudf/cudf/_lib/CMakeLists.txt | 5 -- python/cudf/cudf/_lib/exception_handler.hpp | 80 --------------------- python/cudf/cudf/_lib/exception_handler.pxd | 66 ++++++++++++++++- python/cudf_kafka/pyproject.toml | 2 +- python/custreamz/pyproject.toml | 4 +- python/dask_cudf/pyproject.toml | 2 +- 6 files changed, 69 insertions(+), 90 deletions(-) delete mode 100644 python/cudf/cudf/_lib/exception_handler.hpp diff --git a/python/cudf/cudf/_lib/CMakeLists.txt b/python/cudf/cudf/_lib/CMakeLists.txt index ecacf703bac..c25cc6e6dcb 100644 --- a/python/cudf/cudf/_lib/CMakeLists.txt +++ b/python/cudf/cudf/_lib/CMakeLists.txt @@ -63,11 +63,6 @@ rapids_cython_create_modules( LINKED_LIBRARIES "${linked_libraries}" ASSOCIATED_TARGETS cudf ) -# All modules need to include the header containing the exception handler. -foreach(target IN LISTS RAPIDS_CYTHON_CREATED_TARGETS) - target_include_directories(${target} PRIVATE ${CMAKE_CURRENT_LIST_DIR}) -endforeach() - target_link_libraries(strings_udf cudf_strings_udf) # TODO: Finding NumPy currently requires finding Development due to a bug in CMake. This bug was diff --git a/python/cudf/cudf/_lib/exception_handler.hpp b/python/cudf/cudf/_lib/exception_handler.hpp deleted file mode 100644 index 8daffddd7bd..00000000000 --- a/python/cudf/cudf/_lib/exception_handler.hpp +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (c) 2023, NVIDIA CORPORATION. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include -#include -#include - -namespace cudf_python { -namespace exceptions { - -/** - * @brief Exception handler to map C++ exceptions to Python ones in Cython - * - * This exception handler extends the base exception handler provided by - * Cython (https://github.com/cython/cython/blob/master/Cython/Utility/CppSupport.cpp#L9). - * In addition to the exceptions that Cython itself supports, this file adds support - * for additional exceptions thrown by libcudf that need to be mapped to specific Python - * exceptions. - * - * Since this function interoperates with Python's exception state, it does not throw - * any C++ exceptions. - */ -void cudf_exception_handler() -{ - // Catch a handful of different errors here and turn them into the - // equivalent Python errors. - try { - if (PyErr_Occurred()) - ; // let the latest Python exn pass through and ignore the current one else - throw; - } catch (const std::bad_alloc& exn) { - PyErr_SetString(PyExc_MemoryError, exn.what()); - } catch (const std::bad_cast& exn) { - PyErr_SetString(PyExc_TypeError, exn.what()); - } catch (const std::domain_error& exn) { - PyErr_SetString(PyExc_ValueError, exn.what()); - } catch (const cudf::data_type_error& exn) { - // Have to catch data_type_error before invalid_argument because it is a subclass - PyErr_SetString(PyExc_TypeError, exn.what()); - } catch (const std::invalid_argument& exn) { - PyErr_SetString(PyExc_ValueError, exn.what()); - } catch (const std::ios_base::failure& exn) { - // Unfortunately, in standard C++ we have no way of distinguishing EOF - // from other errors here; be careful with the exception mask - PyErr_SetString(PyExc_IOError, exn.what()); - } catch (const std::out_of_range& exn) { - // Change out_of_range to IndexError - PyErr_SetString(PyExc_IndexError, exn.what()); - } catch (const std::overflow_error& exn) { - PyErr_SetString(PyExc_OverflowError, exn.what()); - } catch (const std::range_error& exn) { - PyErr_SetString(PyExc_ArithmeticError, exn.what()); - } catch (const std::underflow_error& exn) { - PyErr_SetString(PyExc_ArithmeticError, exn.what()); - // The below is the default catch-all case. - } catch (const std::exception& exn) { - PyErr_SetString(PyExc_RuntimeError, exn.what()); - } catch (...) { - PyErr_SetString(PyExc_RuntimeError, "Unknown exception"); - } -} - -} // namespace exceptions -} // namespace cudf_python diff --git a/python/cudf/cudf/_lib/exception_handler.pxd b/python/cudf/cudf/_lib/exception_handler.pxd index 14ac3bb1d40..4337d8db285 100644 --- a/python/cudf/cudf/_lib/exception_handler.pxd +++ b/python/cudf/cudf/_lib/exception_handler.pxd @@ -1,5 +1,69 @@ # Copyright (c) 2023, NVIDIA CORPORATION. -cdef extern from "exception_handler.hpp" namespace "cudf_python::exceptions": +# See +# https://github.com/cython/cython/blob/master/Cython/Utility/CppSupport.cpp +# for the original Cython exception handler. +cdef extern from *: + """ + #include + #include + #include + #include + + namespace { + + /** + * @brief Exception handler to map C++ exceptions to Python ones in Cython + * + * This exception handler extends the base exception handler provided by + * Cython. In addition to the exceptions that Cython itself supports, this + * file adds support for additional exceptions thrown by libcudf that need + * to be mapped to specific Python exceptions. + * + * Since this function interoperates with Python's exception state, it + * does not throw any C++ exceptions. + */ + void cudf_exception_handler() + { + // Catch a handful of different errors here and turn them into the + // equivalent Python errors. + try { + if (PyErr_Occurred()) + ; // let latest Python exn pass through and ignore the current one + throw; + } catch (const std::bad_alloc& exn) { + PyErr_SetString(PyExc_MemoryError, exn.what()); + } catch (const std::bad_cast& exn) { + PyErr_SetString(PyExc_TypeError, exn.what()); + } catch (const std::domain_error& exn) { + PyErr_SetString(PyExc_ValueError, exn.what()); + } catch (const cudf::data_type_error& exn) { + // Catch subclass (data_type_error) before parent (invalid_argument) + PyErr_SetString(PyExc_TypeError, exn.what()); + } catch (const std::invalid_argument& exn) { + PyErr_SetString(PyExc_ValueError, exn.what()); + } catch (const std::ios_base::failure& exn) { + // Unfortunately, in standard C++ we have no way of distinguishing EOF + // from other errors here; be careful with the exception mask + PyErr_SetString(PyExc_IOError, exn.what()); + } catch (const std::out_of_range& exn) { + // Change out_of_range to IndexError + PyErr_SetString(PyExc_IndexError, exn.what()); + } catch (const std::overflow_error& exn) { + PyErr_SetString(PyExc_OverflowError, exn.what()); + } catch (const std::range_error& exn) { + PyErr_SetString(PyExc_ArithmeticError, exn.what()); + } catch (const std::underflow_error& exn) { + PyErr_SetString(PyExc_ArithmeticError, exn.what()); + // The below is the default catch-all case. + } catch (const std::exception& exn) { + PyErr_SetString(PyExc_RuntimeError, exn.what()); + } catch (...) { + PyErr_SetString(PyExc_RuntimeError, "Unknown exception"); + } + } + + } // anonymous namespace + """ cdef void cudf_exception_handler() diff --git a/python/cudf_kafka/pyproject.toml b/python/cudf_kafka/pyproject.toml index 13983db5775..d356399f5e8 100644 --- a/python/cudf_kafka/pyproject.toml +++ b/python/cudf_kafka/pyproject.toml @@ -21,7 +21,7 @@ authors = [ license = { text = "Apache 2.0" } requires-python = ">=3.9" dependencies = [ - "cudf==23.6.*", + "cudf==23.8.*", ] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`. [project.optional-dependencies] diff --git a/python/custreamz/pyproject.toml b/python/custreamz/pyproject.toml index 13f6fafa5d7..c174f28b8c0 100644 --- a/python/custreamz/pyproject.toml +++ b/python/custreamz/pyproject.toml @@ -19,8 +19,8 @@ license = { text = "Apache 2.0" } requires-python = ">=3.9" dependencies = [ "confluent-kafka>=1.9.0,<1.10.0a0", - "cudf==23.6.*", - "cudf_kafka==23.6.*", + "cudf==23.8.*", + "cudf_kafka==23.8.*", "streamz", ] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`. classifiers = [ diff --git a/python/dask_cudf/pyproject.toml b/python/dask_cudf/pyproject.toml index 1b060b7dfb1..da61fa4530a 100644 --- a/python/dask_cudf/pyproject.toml +++ b/python/dask_cudf/pyproject.toml @@ -39,7 +39,7 @@ dynamic = ["entry-points"] [project.optional-dependencies] test = [ - "dask-cuda==23.6.*", + "dask-cuda==23.8.*", "numba>=0.57", "pytest", "pytest-cov",