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

Add tests of dataIdContainer exceptions

    XMLWordPrintable

Details

    • 0.25
    • AP F18-2
    • Alert Production

    Description

      As part of sullivan's demo on using the new coverage system, we picked places lacking in coverage to add coverage to. I chose DataIdContainer in pipe_base/argumentParser.py, which didn't have a test suite dedicated to it, and had a lot of untested exception handling. This ticket is to clean that up.

      Attachments

        Issue Links

          Activity

            rowen: do you mind reviewing this small change? It adds a handful of tests of exception handling to DataIdContainer. There's a larger block of code that I'm not testing here because I don't understand how castDataIds is supposed to behave (somehow self.idList needs to get set to something ahead of time). A small improvement is better than nothing, anyway.

            Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/28229/pipeline

            Parejkoj John Parejko added a comment - rowen : do you mind reviewing this small change? It adds a handful of tests of exception handling to DataIdContainer. There's a larger block of code that I'm not testing here because I don't understand how castDataIds is supposed to behave (somehow self.idList needs to get set to something ahead of time). A small improvement is better than nothing, anyway. Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/28229/pipeline
            rowen Russell Owen added a comment -

            I'll have a look. For the record: dataIdList is a list of butler data IDs, which are simply dicts of key=value pairs. All values in the dicts start out as strings, because that is how they are read from the command line. Casting is used to convert keys to other types as needed (int is fairly common, for example "visit" should have an integer value). Clearly the documentation could use improvement.

            rowen Russell Owen added a comment - I'll have a look. For the record: dataIdList is a list of butler data IDs, which are simply dicts of key=value pairs. All values in the dicts start out as strings, because that is how they are read from the command line. Casting is used to convert keys to other types as needed (int is fairly common, for example "visit" should have an integer value). Clearly the documentation could use improvement.

            Looks good. Thank you for doing this. I filed DM-15162 to improve the docs.

            I had one minor request on github and will add another here: please use unittest.mock (which is in the standard library) instead of mock (a 3rd party package).

            rowen Russell Owen added a comment - Looks good. Thank you for doing this. I filed DM-15162 to improve the docs. I had one minor request on github and will add another here: please use unittest.mock (which is in the standard library) instead of mock (a 3rd party package).
            Parejkoj John Parejko added a comment -

            I'm confused: I'm using `unittest.mock`.

            Parejkoj John Parejko added a comment - I'm confused: I'm using `unittest.mock`.
            rowen Russell Owen added a comment -

            I was confused. You're right. Sorry.

            rowen Russell Owen added a comment - I was confused. You're right. Sorry.
            Parejkoj John Parejko added a comment -

            Thanks. Merged and Done.

            Parejkoj John Parejko added a comment - Thanks. Merged and Done.

            People

              Parejkoj John Parejko
              Parejkoj John Parejko
              Russell Owen
              Ian Sullivan, John Parejko, John Swinbank, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.