Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-9435

Document uses of pex::exceptions

    XMLWordPrintable

    Details

    • Story Points:
      6
    • Sprint:
      Alert Production S17 - 6, AP S18-3, AP S18-5
    • Team:
      Alert Production

      Description

      I cannot find any documents on what to use our current pex::exceptions for. Many of them are similar to the std::c++ exceptions, but even having links to the "matching" C++ doc for each exception would be a start. Some of our exceptions have no std:: counterpart. Either way, it's often unclear at the C++ layer what one should raise. Even a single sentence each would be a good start.

      If this documentation does exist, I can't readily find it in our doxygen or source code.

      Adding a large watcher list, since when I brought this up on slack, Jim Bosch said

      ...first someone has to actually decide what those uses are, and the list of people who feel empowered to decide that is very small.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Pim Schellart [X], I replied on the PR. In particular, any suggestions for how to clarify the recommendations about py::index_error and py::key_error would be greatly appreciated.

            Show
            krzys Krzysztof Findeisen added a comment - Pim Schellart [X] , I replied on the PR. In particular, any suggestions for how to clarify the recommendations about py::index_error and py::key_error would be greatly appreciated.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            See comments on PR.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - See comments on PR.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Pim Schellart [X], would you be willing to review this? The actual RFC-448 changes are about 130 lines, but I took the liberty of doing a few style fixes like clang-format as well.

            Show
            krzys Krzysztof Findeisen added a comment - Pim Schellart [X] , would you be willing to review this? The actual RFC-448 changes are about 130 lines, but I took the liberty of doing a few style fixes like clang-format as well.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            For `IndexError` we should just throw `py::index_error` in the wrappers. If we need it higher up in C++ we can easily change the exception translator in `pex_exceptions`.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - For `IndexError` we should just throw `py::index_error` in the wrappers. If we need it higher up in C++ we can easily change the exception translator in `pex_exceptions`.
            Hide
            jbosch Jim Bosch added a comment -

            Note that we've actually used that functionality quite a bit already to wrap our custom C++ exceptions - I think Pim Schellart [X] may have done most of the implementation for it

            So I think the real question is what those C++ exceptions should be translated to. It's a shame that subclassing IndexError isn't enough for Python in some contexts, but maybe that's a sign we should have C++ exceptions that mirror the Python built-ins, and just translate directly to those.

            Show
            jbosch Jim Bosch added a comment - Note that we've actually used that functionality quite a bit already to wrap our custom C++ exceptions - I think Pim Schellart [X] may have done most of the implementation for it So I think the real question is what those C++ exceptions should be translated to. It's a shame that subclassing IndexError isn't enough for Python in some contexts, but maybe that's a sign we should have C++ exceptions that mirror the Python built-ins, and just translate directly to those.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Possibly useful for getting around problems like IndexError: http://pybind11.readthedocs.io/en/master/advanced/exceptions.html#registering-custom-translators (note that by default most C++ exceptions get wrapped as ValueError).

            Show
            krzys Krzysztof Findeisen added a comment - Possibly useful for getting around problems like IndexError : http://pybind11.readthedocs.io/en/master/advanced/exceptions.html#registering-custom-translators (note that by default most C++ exceptions get wrapped as ValueError ).
            Hide
            jbosch Jim Bosch added a comment -

            For sequence indexing errors, we should try to throw Python's built-in IndexError (not just a subclass) whenever possible; see DM-9715.

            Show
            jbosch Jim Bosch added a comment - For sequence indexing errors, we should try to throw Python's built-in IndexError (not just a subclass) whenever possible; see DM-9715 .

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Pim Schellart [X] (Inactive)
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Jonathan Sick, Kian-Tat Lim, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.