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

enum constants in daf_base.DateTime can cause confusion

    XMLWordPrintable

    Details

      Description

      Whilst looking at DM-7742 I started to wonder why the DateTime constructor did not complain when it was given an MJD time system instead of a UTC time scale.

      It seems we have two issues with DateTime that can lead to subtle bugs not being caught:

      1. Both the TimeScale and DateSystem enums share values in Python land. This means that in Python you can pass in an MJD but C++ will treat it as UTC. We should ensure that both these enums convert to different sets of integers in Python so that we can trap system/scale confusion.
      2. The DateTime constructor does not check all cases. It assumes that if the scale is not TT or TAI then the user probably meant UTC. The code in DateTime should never use an else to assume UTC but should always check and trigger and exception if the value is not recognized.

        Attachments

          Issue Links

          is triggered by

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-7742 constructCalibs.py calls dafBase.DateTime without a timescale argument

          • Done
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-5503 Implement single interface to sanitized exposure metadata

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            rowen Russell Owen added a comment -

            I will fix this as part of DM-5503

            Show
            rowen Russell Owen added a comment - I will fix this as part of DM-5503
            Hide
            Permalink
            rowen Russell Owen added a comment -

            By the way: Tim Jenness's statement about the DateTime constructor is incorrect. The constructor in question does reject invalid values of the time scale argument, it just does it deep down. There is an early if/else to handle Z or no Z on ISO strings, but that is not the main test. (Even so, the if/else should be "if system==UTC then require Z" instead of the way it is written now, and I will fix that as part of this ticket.)

            Show
            rowen Russell Owen added a comment - By the way: Tim Jenness 's statement about the DateTime constructor is incorrect. The constructor in question does reject invalid values of the time scale argument, it just does it deep down. There is an early if/else to handle Z or no Z on ISO strings, but that is not the main test. (Even so, the if/else should be "if system==UTC then require Z" instead of the way it is written now, and I will fix that as part of this ticket.)
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Being fixed as part of DM-5503

            Show
            rowen Russell Owen added a comment - Being fixed as part of DM-5503
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Fixed as part of DM-5503

            Show
            rowen Russell Owen added a comment - Fixed as part of DM-5503

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              tjenness Tim Jenness
              Watchers:
              Kian-Tat Lim, Paul Price, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.