DM-17427: objectMasks: update parser to handle Goulding mask format #280
Conversation
|
|
||
| if _type == "box": | ||
| width = convertToAngle(param1, param1Unit, "width", fileName, lineNo) | ||
| height = convertToAngle(param2, param2Unit, "height", fileName, lineNo) | ||
| angle = convertToAngle(param3, param3Unit, "angle", fileName, lineNo) | ||
| if param3 is not None: | ||
| angle = convertToAngle(param3, param3Unit, "angle", fileName, lineNo) |
There was a problem hiding this comment.
You are basically doing work to optionally read in an angle if supplied but below there can only be an angle of zero, and it is only output as a warning. Should that be stronger than a warning as it is not going to do as the user requests? Or should we just take rotation out all the way?
There was a problem hiding this comment.
I went to change this to an error, but then noticed that it actually does end up throwing an error because of this. It's putting out warnings as it reads each line, and then if there were any problems it throws an error when it's done reading. I think failing late here is the right thing to do, as it allows the user to fix all the bad lines rather than one by one (nicely done, @RobertLuptonTheGood!).
Andy Goulding has provided new masks based on Gaia DR2, and with a better tuned size; but the format is subtly different: * The rotation angle of a 'box' is not specified (it’s supposed to be zero anyway). * The 'ID:' string is replaced with 'Gaia DR2'. * Floating point values use scientific notation in places. We've decided that (apart from the lack of "ID:", which we want in order to be explicit) it's best to update the parser rather than fix the mask files: the differences from what we have been expecting are reasonable, and fixing the parser now means we may not have to fix it much for the next version. Expanded the regex to support the new format, defaulting the rotation angle of boxes to 0. Added comments to the regex so it's easier to understand.
PaulPrice
force-pushed
the
tickets/DM-17427
branch
from
833810d
to
411cdf7
Compare
Andy Goulding has provided new masks based on Gaia DR2, and with a
better tuned size; but the format is subtly different:
zero anyway).
We've decided that (apart from the lack of "ID:", which we want in
order to be explicit) it's best to update the parser rather than fix
the mask files: the differences from what we have been expecting are
reasonable, and fixing the parser now means we may not have to fix
it much for the next version.
Expanded the regex to support the new format, defaulting the
rotation angle of boxes to 0. Added comments to the regex so it's
easier to understand.