Hi Perry –
I've added some review comments to this GitHub pull request. To summarize:
I am basically happy with the way you've ported the algorithm itself to the new framework. There are a few minor issues of code formatting which you should address before merging, and please make sure that any work which has to be performed in future is recorded in JIRA rather than marked as FIXME in the source.
I'm a bit more concerned about the state of the tests – it seems that there are quite a few checks which have been skipped without explanation, and a lot of duplication has been introduced. It may be that there are perfectly good reasons, but they aren't obvious from reading the code, but (similarly to the above) just commenting things out isn't a good answer – if it requires more work to make the tests pass, we should record what's required and why it can't be done now in JIRA.
As you note in the comment above, the tests fail if display=True. The correct fix is to run base_SkyCoord, as you suggest, but be aware that you need to add it to the single frame measurement (line 100 of tests/Kron.py) not the forced measurement (line 97).
As has been mentioned, you'll need to rebase your branch so that it has a logical & linear history before you can merge to master.
Finally, I'm a bit confused by the sequencing of the various issues here. According to the comments on this story, this work shouldn't have been done until the various HSC side fixes had been committed by Jim in
DM-1783. However, it looks as though you branched from master before DM-1783 landed on 26 January. I don't think that invalidates your work here, but it might mean you have more work to do to merge your changes – certainly, your branch doesn't currently cleanly rebase onto master.