# 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
• Story Points:
0.5
• Team:
Data Release Production

#### 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.

#### Activity

Hide
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 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
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?

Show
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
Robert Lupton added a comment -

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

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

#### People

Assignee:
Robert Lupton
Reporter:
Robert Lupton
Reviewers:
John Swinbank
Watchers:
John Swinbank, Robert Lupton