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

Fix to DM-2883 isn't quite right

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      The fix to DM-2883 (remove illegal PVi_j cards) isn't quite right, and the error was masked by a piece of code elsewhere that duplicated the functionality.

      The issues is that while PV1_[1-4] cards are indeed valid, the ones that SCAMP writes are not. So we should remove them too, if there are any other SCAMP TPV coefficients.

      The masking code was a unilateral removal of PVi_j cards dating back years.

        Attachments

          Issue Links

          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-3196 makeWcs() chokes on decam images in 10.1

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-2883 wcslib is unable to read PTF headers with PV1_{1..16} cards

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            rhl Robert Lupton added a comment - 

            $ git diff --stat origin/master..
            		 
             src/image/Wcs.cc     | 15 ---------------
            		 
             src/image/makeWcs.cc | 12 +++++++++---
            		 
             2 files changed, 9 insertions(+), 18 deletions(-)

            Show
            rhl Robert Lupton added a comment - $ git diff --stat origin/master.. src/image/Wcs.cc | 15 --------------- src/image/makeWcs.cc | 12 +++++++++--- 2 files changed, 9 insertions(+), 18 deletions(-)
            Hide
            Permalink
            swinbank John Swinbank added a comment -

            The code as written won't compile on GCC 4.4, which doesn't support range-based for. That's an easy fix, though. I made a couple of other minor comments in the GitHub PR.

            The previous code renamed, rather than simply removed, the troublesome header cards. I'm not sure if that was useful. Do you think it's behaviour that's worth preserving?

            How about a test case?

            Show
            swinbank John Swinbank added a comment - The code as written won't compile on GCC 4.4, which doesn't support range-based for. That's an easy fix, though. I made a couple of other minor comments in the GitHub PR. The previous code renamed, rather than simply removed, the troublesome header cards. I'm not sure if that was useful. Do you think it's behaviour that's worth preserving? How about a test case?
            Hide
            Permalink
            rhl Robert Lupton added a comment -

            Ran buildbot (successfully), merged, and pushed to master

            Show
            rhl Robert Lupton added a comment - Ran buildbot (successfully), merged, and pushed to master

              People

              Assignee:
              rhl Robert Lupton
              Reporter:
              rhl Robert Lupton
              Reviewers:
              John Swinbank
              Watchers:
              John Swinbank, Robert Lupton
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.