diff --git a/docs/source/conf.py b/docs/source/conf.py index 812a8ddf9..e6d7453d3 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -26,6 +26,10 @@ bibtex_bibfiles = ["libsemigroups.bib"] autosummary_generate = True +# We set this to True here, but remove "libsemigroups_pybind11\..*" from the doc +# everywhere. This is done so we still get the submodule names, but not the +# global module name. A nicer, but more involved solution, could use some Sphinx +# magic as done in https://stackoverflow.com/a/72658470/15278419. add_module_names = True templates_path = ["_templates"] diff --git a/docs/source/data-structures/misc/libsemigroups-error.rst b/docs/source/data-structures/misc/libsemigroups-error.rst index 497eeebc8..d10c9c4e8 100644 --- a/docs/source/data-structures/misc/libsemigroups-error.rst +++ b/docs/source/data-structures/misc/libsemigroups-error.rst @@ -11,11 +11,15 @@ Exceptions ========== This page describes the custom error type used in ``libsemigroups_pybind11``, -namely :any:`LibsemigroupsError`. Other built-in exceptions, such as -:any:`ValueError` and :any:`TypeError`, may also be raised in this project. See -the `Python documentation `_ +namely :any:`LibsemigroupsError`, and related functions. Other built-in +exceptions, such as :any:`ValueError` and :any:`TypeError`, may also be raised +in this project. See the +`Python documentation `_ for further details. +Full API +-------- + .. autoexception:: LibsemigroupsError :show-inheritance: @@ -27,14 +31,14 @@ for further details. >>> from libsemigroups_pybind11 import FroidurePin, Perm >>> gens = [Perm([3, 0, 1, 2]), Perm([1, 2, 0, 3]), Perm([2, 1, 0, 3])] - >>> S = FroidurePin(gens[0]) - >>> S.add_generators(gens[1:]) + >>> S = FroidurePin(gens) + >>> S >>> S.generator(3) # Bad: there are only three generators Traceback (most recent call last): ... - _libsemigroups_pybind11.LibsemigroupsError: generator index out of bounds, expected value in [0, 3), got 3 + LibsemigroupsError: generator index out of bounds, expected value in [0, 3), got 3 .. note:: @@ -43,3 +47,5 @@ for further details. but you think it should, please let us known by opening an issue on the `issue tracker `_. + +.. autofunction:: error_message_with_prefix \ No newline at end of file diff --git a/docs/source/index.rst b/docs/source/index.rst index effb8fb50..3f701a895 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -50,6 +50,63 @@ See the installation instructions: install changelog +The structure of the module +~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The Python module ``libsemigroups_pybind11`` was designed to mirror the C++ +library ``libsemigroups`` as closely as possible, whilst navigating the +fundamental difference between Python and C++. This is done with the help of the +tool `pybind11 `_. + +For various implementational reasons (mostly related to Python's lack of an +analogue for C++'s templating system), the Python code exposed by ``pybind11`` +is less streamlined and concise than the C++ library. To try and address this, +the authors of ``libsemigroups_pybind11`` have further wrapped the Python code +produced by ``pybind11`` to make the Python and C++ user experience as similar +as possible. + +The Python bindings of the ``libsemigroups`` code produced by ``pybind11`` +reside in an intermediate module called ``_libsemigroups_pybind11`` (note the +leading underscore), and the public-facing nicely wrapped code resides in this +module — ``libsemigroups_pybind11``. + +Should this impact the way you, the user, use ``libsemigroups_pybind11``? For +the most part, no. It should be possible to use ``libsemigroups_pybind11`` in +the ways documented on this site without the knowledge that +``_libsemigroups_pybind11`` even exists. The notable exceptions to this relate +to type names and error messages. A lot of the code in +``libsemigroups_pybind11`` has been imported from ``_libsemigroups_pybind11``, +and this is visible in qualified type names. For example: + +.. doctest:: python + + >>> from libsemigroups_pybind11 import WordGraph + >>> WordGraph + + +Additionally, some functions or classes in the ``_libsemigroups_pybind11`` +module have additional prefixes and suffixes relative to their +``libsemigroups_pybind11`` counterpart. These usually relate to the module that +the function or class is contained within or a type the function or class is +defined upon. These may appear in error messages. For example: + +.. doctest:: python + + >>> from libsemigroups_pybind11 import aho_corasick, AhoCorasick + >>> ac = AhoCorasick() + >>> aho_corasick.add_word(ac, False) + Traceback (most recent call last): + ... + TypeError: aho_corasick_add_word(): incompatible function arguments. The following argument types are supported: + 1. (ac: _libsemigroups_pybind11.AhoCorasick, w: list[int]) -> int + 2. (ac: _libsemigroups_pybind11.AhoCorasick, w: str) -> int + + Invoked with: , False + +The authors of ``libsemigroups_pybind11`` have gone to a lot of effort to +try and make error messages clear, specific and intelligible; however, if there +you encounter any errors with unclear messages, please raise this on the +`issue tracker `_. + .. toctree:: :caption: Data Structures :hidden: diff --git a/etc/replace-strings-in-doc.py b/etc/replace-strings-in-doc.py index fc4e8f498..df9f06650 100755 --- a/etc/replace-strings-in-doc.py +++ b/etc/replace-strings-in-doc.py @@ -18,8 +18,7 @@ args = parser.parse_args() replacements = { - r"_libsemigroups_pybind11.": "", - r"libsemigroups_pybind11\.": "", + r'(?<=)_?libsemigroups_pybind11\.': "", } html_glob = "docs/_build/html/**/*.html" diff --git a/src/action.cpp b/src/action.cpp index 6f93f9bab..db95199ff 100644 --- a/src/action.cpp +++ b/src/action.cpp @@ -147,7 +147,7 @@ Increase the capacity to a value that is greater or equal to *val*. :param val: new capacity of an action instance. :type val: int -:raises ValueError: if ``val`` is too large. +:raises MemoryError: if ``val`` is too large. :complexity: At most linear in the :any:`size()` of the :py:class:`Action`. diff --git a/src/adapters.cpp b/src/adapters.cpp index 6b52c22c2..8aac1ed15 100644 --- a/src/adapters.cpp +++ b/src/adapters.cpp @@ -102,7 +102,7 @@ This call operator returns the image of *pt* acted on by *x*. :rtype: Point :raises TypeError: - If the wrapped C++ type of the sample objects passed via *element* and + If the wrapped C++ type of the sample objects passed via *x* and *point* are not the same as the wrapped types of the arguments in any invocation of the call operator. For example, if *point* is ``PPerm([], [], 256)``, then the underlying C++ type uses 8-bit integers to store image @@ -171,7 +171,7 @@ This call operator returns the image of *pt* acted on by *x*. :rtype: Point :raises TypeError: - If the wrapped C++ type of the sample objects passed via *element* and + If the wrapped C++ type of the sample objects passed via *x* and *point* are not the same as the wrapped types of the arguments in any invocation of the call operator. For example, if *point* is ``PPerm([], [], 256)``, then the underlying C++ type uses 8-bit integers to store image diff --git a/src/dot.cpp b/src/dot.cpp index 0b28374c7..3f61b73ba 100644 --- a/src/dot.cpp +++ b/src/dot.cpp @@ -52,10 +52,9 @@ render it with the `Graphviz `_ installation on your system. )pbdoc"); dot.def("__repr__", py::overload_cast(&to_human_readable_repr)); - dot.def_property( + dot.def_property_readonly( "colors", [](Dot const& self) { return Dot::colors; }, - []() {}, // TODO raise exception R"pbdoc( An array of default HTML/hex colours. )pbdoc"); @@ -357,10 +356,9 @@ This nested class represents a node in the represented graph. )pbdoc"); n.def("__repr__", py::overload_cast(&to_human_readable_repr)); - n.def_property( + n.def_property_readonly( "attrs", [](Dot::Node& self) { return self.attrs; }, - []() {}, R"pbdoc( Read-only dictionary containing the attributes of the node. )pbdoc"); @@ -397,10 +395,9 @@ Instances of this nested class represents an edge in the represented graph. )pbdoc"); e.def("__repr__", py::overload_cast(&to_human_readable_repr)); - e.def_property( + e.def_property_readonly( "attrs", [](Dot::Edge& self) { return self.attrs; }, - []() {}, R"pbdoc( Read-only dictionary containing containing the attributes of the :any:`Edge`. )pbdoc"); diff --git a/src/errors.cpp b/src/errors.cpp index a66918ca1..4ad0df039 100644 --- a/src/errors.cpp +++ b/src/errors.cpp @@ -55,27 +55,55 @@ namespace libsemigroups { } void init_error(py::module& m) { - // TODO this doesn't seem to properly catch all LibsemigroupsExceptions, - // particularly on macOS. This may have been resolved in pybind11 2.12.0 - static py::exception exc( - m, "LibsemigroupsError", PyExc_RuntimeError); + // Using the GIL safe call below rather than simply having a static + // py::exception is recommended in the pybind11 doc. + PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store + exc_storage; + exc_storage.call_once_and_store_result([&]() { + return py::exception( + m, "LibsemigroupsError", PyExc_RuntimeError); + }); py::register_exception_translator([](std::exception_ptr p) { try { if (p) { std::rethrow_exception(p); } } catch (LibsemigroupsException const& e) { - exc(formatted_error_message(e).c_str()); - } catch (py::stop_iteration const& e) { - throw e; - } catch (std::runtime_error const& e) { - exc(formatted_error_message(e).c_str()); + py::set_error(exc_storage.get_stored(), + formatted_error_message(e).c_str()); } }); - // TODO: Doc + m.def("error_message_with_prefix", - py::overload_cast<>(&error_message_with_prefix)); + py::overload_cast<>(&error_message_with_prefix), + R"pbdoc( +Return whether :any:`LibsemigroupsError` messages have a C++ prefix. + +Since ``libsemigroups_pybind11`` is built on top of the C++ library +``libsemigroups``, many of the errors thrown in ``libsemigroups_pybind11`` +emanate from C++. This function returns whether :any:`LibsemigroupsError` +messages contain a prefix that indicates which C++ function raised the +exception. + +:return: Whether :any:`LibsemigroupsError` messages contain a prefix about the + C++ function that raised the exception. +:rtype: bool +)pbdoc"); m.def("error_message_with_prefix", - py::overload_cast(&error_message_with_prefix)); + py::overload_cast(&error_message_with_prefix), + py::arg("val"), + R"pbdoc( +Specify whether :any:`LibsemigroupsError` messages have a C++ prefix. + +Since ``libsemigroups_pybind11`` is built on top of the C++ library +``libsemigroups``, many of the errors thrown in ``libsemigroups_pybind11`` +emanate from C++. This function specifies whether :any:`LibsemigroupsError` +messages should contain a prefix that indicates which C++ function raised the +exception. By default, this information is not included. + +:param val: Whether :any:`LibsemigroupsError` messages should contain a prefix + about the C++ function that raised the exception. +:type val: bool +)pbdoc"); } } // namespace libsemigroups diff --git a/src/froidure-pin.cpp b/src/froidure-pin.cpp index a822c3a60..fd8c318c7 100644 --- a/src/froidure-pin.cpp +++ b/src/froidure-pin.cpp @@ -132,6 +132,10 @@ Copy a :any:`FroidurePin` object. :returns: A copy. :rtype: FroidurePin )pbdoc"); + // This function should really throw a ValueError if the degree of x is + // incompatible with the existing degree, but this doesn't get detected at + // the Python level, so a LibsemigroupsError is thrown instead. It would + // be possible to intercept this, but it probably isn't worth the effort. thing.def("add_generator", &FroidurePin_::add_generator, py::arg("x"), @@ -166,7 +170,7 @@ elements than before (whether it is fully enumerating or not). :returns: ``self``. :rtype: FroidurePin -:raises ValueError: +:raises LibsemigroupsError: if the degree of *x* is incompatible with the existing degree (if any). :raises TypeError: @@ -199,7 +203,7 @@ See :any:`add_generator` for a detailed description. :raises TypeError: if any item in *gens* is not of the same type as the existing generators (if any). -:raises ValueError: +:raises LibsemigroupsError: if the degree of any item in *gens* is incompatible with the existing degree (if any). )pbdoc"); diff --git a/src/libsemigroups_pybind11/action.py b/src/libsemigroups_pybind11/action.py index c3947278d..26f3da1d5 100644 --- a/src/libsemigroups_pybind11/action.py +++ b/src/libsemigroups_pybind11/action.py @@ -209,7 +209,7 @@ def __init__( if _to_cxx(self) is not None: return if len(args) != 0: - raise ValueError( + raise TypeError( f"expected 0 positional arguments, but found {len(args)}" ) if not isinstance(generators, list): diff --git a/src/libsemigroups_pybind11/adapters.py b/src/libsemigroups_pybind11/adapters.py index 8df60bc7e..07673b4d9 100644 --- a/src/libsemigroups_pybind11/adapters.py +++ b/src/libsemigroups_pybind11/adapters.py @@ -68,9 +68,7 @@ def __init__(self: _Self, *args, point=None, element=None) -> None: if _to_cxx(self) is not None: return if len(args) != 0: - raise ValueError( - f"expected 0 positional arguments, but found {len(args)}" - ) + raise TypeError(f"expected 0 positional arguments, but found {len(args)}") self.py_template_params = ( type(_to_cxx(element)), type(_to_cxx(point)), diff --git a/src/libsemigroups_pybind11/detail/cxx_wrapper.py b/src/libsemigroups_pybind11/detail/cxx_wrapper.py index d44661ef1..b38a4e1a3 100644 --- a/src/libsemigroups_pybind11/detail/cxx_wrapper.py +++ b/src/libsemigroups_pybind11/detail/cxx_wrapper.py @@ -81,13 +81,13 @@ def __init__( for kwarg in required_kwargs: if kwarg not in kwargs: - raise ValueError( + raise TypeError( f'required keyword argument "{kwarg}" not found, ' f"found {tuple(kwargs.keys())} instead" ) for kwarg in kwargs: if kwarg not in required_kwargs and kwarg not in optional_kwargs: - raise ValueError( + raise TypeError( f'unexpected keyword argument "{kwarg}", ' f"required keyword arguments are {required_kwargs} " f"and optional keyword arguments are {optional_kwargs}" diff --git a/src/libsemigroups_pybind11/froidure_pin.py b/src/libsemigroups_pybind11/froidure_pin.py index 69f401f37..2bb59a69f 100644 --- a/src/libsemigroups_pybind11/froidure_pin.py +++ b/src/libsemigroups_pybind11/froidure_pin.py @@ -162,7 +162,7 @@ def __init__(self: _Self, *args) -> None: if _to_cxx(self) is not None: return if len(args) == 0: - raise ValueError("expected at least 1 argument, found 0") + raise TypeError("expected at least 1 argument, found 0") if isinstance(args[0], list) and len(args) == 1: gens = args[0] diff --git a/src/libsemigroups_pybind11/konieczny.py b/src/libsemigroups_pybind11/konieczny.py index 51d527dea..860d69b70 100644 --- a/src/libsemigroups_pybind11/konieczny.py +++ b/src/libsemigroups_pybind11/konieczny.py @@ -130,7 +130,7 @@ def __init__(self: Self, *args) -> None: if _to_cxx(self) is not None: return if len(args) == 0: - raise ValueError("expected at least 1 argument, found 0") + raise TypeError("expected at least 1 argument, found 0") if isinstance(args[0], list) and len(args) == 1: gens = args[0] diff --git a/src/libsemigroups_pybind11/presentation/__init__.py b/src/libsemigroups_pybind11/presentation/__init__.py index b1634b1a1..6c72433b9 100644 --- a/src/libsemigroups_pybind11/presentation/__init__.py +++ b/src/libsemigroups_pybind11/presentation/__init__.py @@ -103,7 +103,7 @@ def __init__(self: Self, *args, **kwargs) -> None: or (len(args) == 1 and len(kwargs) > 0) or len(args) > 1 ): - raise ValueError( + raise TypeError( 'expected 1 positional argument or the keyword argument "Word"' f" but found {len(args)} positional arguments, and keywords arguments " f"{tuple(kwargs.keys())}" @@ -123,7 +123,7 @@ def __init__(self: Self, *args, **kwargs) -> None: f"but found {type(args[0])}" ) if isinstance(args[0], list) and not all(isinstance(x, int) for x in args[0]): - raise ValueError("expected the argument to consist of int values") + raise TypeError("expected the argument to consist of int values") if isinstance(args[0], str): self.py_template_params = (str,) if isinstance(args[0], list): diff --git a/src/libsemigroups_pybind11/schreier_sims.py b/src/libsemigroups_pybind11/schreier_sims.py index 6fa7b79b5..07cb943b1 100644 --- a/src/libsemigroups_pybind11/schreier_sims.py +++ b/src/libsemigroups_pybind11/schreier_sims.py @@ -71,7 +71,7 @@ def __init__(self: _Self, *args) -> None: if _to_cxx(self) is not None: return if len(args) == 0: - raise ValueError("expected at least 1 argument, found 0") + raise TypeError("expected at least 1 argument, found 0") if isinstance(args[0], list) and len(args) == 1: gens = args[0] diff --git a/src/libsemigroups_pybind11/stephen.py b/src/libsemigroups_pybind11/stephen.py index e6d9a2c3e..a97706079 100644 --- a/src/libsemigroups_pybind11/stephen.py +++ b/src/libsemigroups_pybind11/stephen.py @@ -73,7 +73,7 @@ def __init__(self: _Self, *args, **kwargs) -> None: return if len(args) != 1: - raise ValueError(f"expected 1 argument, but got {len(args)}") + raise TypeError(f"expected 1 argument, but got {len(args)}") if isinstance(args[0], (_Presentation, _InversePresentation)): self.py_template_params = (type(_to_cxx(args[0])),) diff --git a/src/matrix.cpp b/src/matrix.cpp index 9369855bd..fb887d0e2 100644 --- a/src/matrix.cpp +++ b/src/matrix.cpp @@ -27,7 +27,8 @@ // libsemigroups.... #include // for Hash -#include // for MaxPlusTruncMat, MinPlusTruncMat +#include // for PositiveInfinity, NegativeInf... +#include // for LIBSEMIGROUPS_EXCEPTION #include // for MaxPlusTruncMat, MinPlusTruncMat #include // for string_format, to_string @@ -408,7 +409,7 @@ Construct a matrix from rows. :param rows: the rows of the matrix. :type rows: list[list[int | PositiveInfinity | NegativeInfinity]] -:raise RunTimeError: if *kind* is +:raise TypeError: if *kind* is :py:attr:`MatrixKind.MaxPlusTrunc`, :py:attr:`MatrixKind.MinPlusTrunc`, or :py:attr:`MatrixKind.NTP`. diff --git a/src/report.cpp b/src/report.cpp index eab8e4192..adde609be 100644 --- a/src/report.cpp +++ b/src/report.cpp @@ -23,7 +23,7 @@ #include // libsemigroups_pybind11.... -#include "main.hpp" // for init_error +#include "main.hpp" // for init_detail_report namespace libsemigroups { diff --git a/src/word-range.cpp b/src/word-range.cpp index 329c25443..7bbe7935a 100644 --- a/src/word-range.cpp +++ b/src/word-range.cpp @@ -26,6 +26,7 @@ // libsemigroups.... +#include // for LIBSEMIGROUPS_EXCEPTION #include // for Path #include // for word_type #include // for number_of_words @@ -1423,7 +1424,7 @@ documentation of that function for more details. :returns: The human readable index. :rtype: int -:raises LibsemigroupsError: if *c* is not exactly one character long. +:raises ValueError: if *c* is not exactly one character long. .. seealso:: :any:`words.human_readable_letter` diff --git a/tests/test_schreier_sims.py b/tests/test_schreier_sims.py index 57fcfde1c..39b4ca02d 100644 --- a/tests/test_schreier_sims.py +++ b/tests/test_schreier_sims.py @@ -29,7 +29,7 @@ def check_constructors(gens): ReportGuard(False) # default constructor - with pytest.raises(ValueError): + with pytest.raises(TypeError): SchreierSims() S1 = SchreierSims(gens) diff --git a/tests/test_words.py b/tests/test_words.py index 6acfda46c..440cbe0ab 100644 --- a/tests/test_words.py +++ b/tests/test_words.py @@ -348,5 +348,5 @@ def test_human_readable_letter(): def test_human_readable_index(): assert human_readable_index("\377") == 255 - with pytest.raises(LibsemigroupsError): + with pytest.raises(ValueError): human_readable_index("Ā") # this is character 256