Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_deblender, meas_extensions_scarlet
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Sprint:AP F20-3 (August)
-
Team:Alert Production
-
Urgent?:No
Description
To sourceDeblendTask.py and scarletDeblendTask.py respectively, thereby implementing RFC-717.
Attachments
Issue Links
Activity
No problem, I wanted to make sure that it could make it in the next weekly.
My thinking was just that if someone looks at this ticket in the future then they might be misled about what changes were actually made. I don't see the deprecation warning for meas_extensions_scarlet or pipe_tasks. You never created a PR for those repos and they still seem to reflect the name change, which has been taken out of this ticket, so I assumed that you were just saving that for a future ticket. Or did you jsut forget to push your latest changes?
Fred Moolekamp
added a comment - No problem, I wanted to make sure that it could make it in the next weekly.
My thinking was just that if someone looks at this ticket in the future then they might be misled about what changes were actually made. I don't see the deprecation warning for meas_extensions_scarlet or pipe_tasks . You never created a PR for those repos and they still seem to reflect the name change, which has been taken out of this ticket, so I assumed that you were just saving that for a future ticket. Or did you jsut forget to push your latest changes? Hi Fred — I think maybe it's not obvious from the display on GitHub exactly what changes I made in meas_extensions_scarlet. Try using the “split view” for the diff and looking at deblend.py specifically. You should be seeing something like the changes in the attached screenshot.
In other words, on this ticket I have moved the relevant code in meas_extensions_scarlet, but left a deprecated backwards compatibility shim so the old names will keep working for now.
Ok, I get it now. I think I was expecting you to rename `git mv deblend.py scarletDeblendTask.py` (so that changes could be tracked easier), then create a new `deblend.py` that had the deprecation warning. Since only those few lines changed I didn't even see them when I quickly scrolled through the file.
But no need to fix it now. This is fine.
Fred Moolekamp
added a comment - Ok, I get it now. I think I was expecting you to rename `git mv deblend.py scarletDeblendTask.py` (so that changes could be tracked easier), then create a new `deblend.py` that had the deprecation warning. Since only those few lines changed I didn't even see them when I quickly scrolled through the file.
But no need to fix it now. This is fine. I was expecting you to rename `git mv deblend.py scarletDeblendTask.py` (so that changes could be tracked easier), then create a new `deblend.py` that had the deprecation warning.
I did! But git doesn't explicitly track renames; “copy and edit the original”, and “move and create a new file with the same name as the original” are equivalent as far as it's concerned: you can't reverse-engineer what happened by looking at the diffs.
Anyway, thanks for the review! First Jenkins run failed due to internal Jenkins problems, so I'm waiting for a re-run and will then merge.
Thanks for the quick turnaround!
Are you worried about the confusion between the title of this ticket being "remove", and the fact the file isn't actually removed from meas_extensions_scarlet? If so, would you be happy with me simply tweaking the title of this ticket? Otherwise it seems like a lot of branch manipulation for what are basically tiny changes...