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

Document uses of pex::exceptions

    XMLWordPrintable

    Details

    • Story Points:
      6
    • Epic Link:
    • 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

          is triggered by

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-448 Document intended purposes of LSST exceptions

          • Implemented
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-9428 Update exceptions tutorial-level documentation for pybind11

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-18855 Pex exceptions TypeError should not inherit from RuntimeError

          • Done

            Activity

            Ascending order - Click to sort in descending order
            No builds found.
            Parejkoj John Parejko created issue -
            Parejkoj John Parejko made changes -
            Field Original Value New Value
            Link This issue relates to DM-9428 [ DM-9428 ]
            tjenness Tim Jenness made changes -
            Watchers Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Russell Owen, Simon Krughoff [ Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Russell Owen, Simon Krughoff ] Jim Bosch, John Parejko, John Swinbank, Jonathan Sick, Kian-Tat Lim, Krzysztof Findeisen, Russell Owen, Simon Krughoff [ Jim Bosch, John Parejko, John Swinbank, Jonathan Sick, Kian-Tat Lim, Krzysztof Findeisen, Russell Owen, Simon Krughoff ]
            Hide
            Permalink
            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 .
            Hide
            Permalink
            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
            Permalink
            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
            Permalink
            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`.
            krzys Krzysztof Findeisen made changes -
            Assignee Krzysztof Findeisen [ krzys ]
            krzys Krzysztof Findeisen made changes -
            Epic Link DM-9680 [ 30785 ]
            krzys Krzysztof Findeisen made changes -
            Team Alert Production [ 10300 ]
            krzys Krzysztof Findeisen made changes -
            Story Points 6
            Parejkoj John Parejko made changes -
            Labels SciencePipelines PairCoding SciencePipelines
            krzys Krzysztof Findeisen made changes -
            Labels PairCoding SciencePipelines SciencePipelines
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6 [ 616 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krughoff Simon Krughoff made changes -
            Sprint Alert Production S17 - 6 [ 616 ] Alert Production S17 - 6, Alert Production F17 - 7 [ 616, 626 ]
            krughoff Simon Krughoff made changes -
            Rank Ranked higher
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6, Alert Production F17 - 7 [ 616, 626 ] Alert Production S17 - 6 [ 616 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            swinbank John Swinbank made changes -
            Epic Link DM-9680 [ 30785 ] DM-10068 [ 31628 ]
            swinbank John Swinbank made changes -
            Epic Link DM-10068 [ 31628 ] DM-11798 [ 34281 ]
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6 [ 616 ] Alert Production S17 - 6, Alert Production F17 - 11 [ 616, 644 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6, Alert Production F17 - 11 [ 616, 644 ] Alert Production S17 - 6 [ 616 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked higher
            swinbank John Swinbank made changes -
            Epic Link DM-11798 [ 34281 ] DM-12728 [ 36327 ]
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6 [ 616 ] Alert Production S17 - 6, AP S18-3 [ 616, 683 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue is triggered by RFC-448 [ RFC-448 ]
            swinbank John Swinbank made changes -
            Sprint Alert Production S17 - 6, AP S18-3 [ 616, 683 ] Alert Production S17 - 6, AP S18-3, AP S18-4 [ 616, 683, 684 ]
            krzys Krzysztof Findeisen made changes -
            Sprint Alert Production S17 - 6, AP S18-3, AP S18-4 [ 616, 683, 684 ] Alert Production S17 - 6, AP S18-3, AP S18-5 [ 616, 683, 685 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krzys Krzysztof Findeisen made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            Permalink
            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.
            krzys Krzysztof Findeisen made changes -
            Reviewers Pim Schellart [ pschella ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            Permalink
            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.
            pschella Pim Schellart [X] (Inactive) made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            Permalink
            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.
            krzys Krzysztof Findeisen made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-18855 [ DM-18855 ]

              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:

                  Jenkins

                  No builds found.