add support for translating ctp coordinates

[vsoch]

We identified a bug in #230 where the ctp coordinates are supplied as:

(xmin, ymin, width, height)

but our parser is expecting:

(xmin, ymin, xmax, ymax)

So I think the translation is fairly simple - the first two coordinates (I think) are correct, and we just need to add the width and height to each to derive the xmax and ymax that we expect. We can do this right at parsing (config/utils.py) and then still provide the "same" structured coordinates later.

This means I've added a set of new coordinate types, ctpcoordinates and ctpkeepcoordinates that mirror the coordinates and keepcoordinates but expect this slightly different input format, and updated our default deid.dicom to use them (since I believe all entries in here minus the ones from stanford faculty are from CTP!)

Signed-off-by: vsoch vsoch@users.noreply.github.com

Checklist

• [x] I have commented my code, particularly in hard-to-understand areas
• [x] My changes generate no new warnings
• [x] My code follows the style guidelines of this project

Open questions

Is there a reasonable way to better test this?

[vsoch]

@wetzelj good to go on your end?

[wetzelj]

I've run a couple basic tests - and this looks pretty good.

At first I was a little concerned about the REMOVE functionality, specifically this comment from the discussion on #197. When I tested placing a REMOVE at the end of my recipe, it did indeed remove a field that I had already added. The REMOVE wasn't as I expected from reading the comments on #197. On some further testing, it appears as though a REMOVE following an ADD or JITTER is still removing the field (what I consider current functionality), while a REMOVE following a KEEP or REPLACE ignores the remove (new functionality, but very worthwhile).

That said, I don't know that this necessarily needs to hold up this merge, but should probably be opened as another issue - to thoroughly evaluate and determine how we want these all to work together. What do you think?

[vsoch]

I think I would expect a REMOVE to remove (regardless of previous history) UNLESS someone asked to keep - so I don't see issue with the current functionality.

[vsoch]

I think there are enough good changes here that it's worth merging and releasing, and we can open an issue for further discussion on remove. Thanks @wetzelj !

[wetzelj]

Thank you @vsoch!