Consider moving find_transform into astropy

[eteq]

While thinking about #34 and after some time of keeping an eye out for the use cases, I think there are a number of places in the wider Astropy ecosystem where the asterism-matching machinery in astroalign might be useful on its own (i.e., not part of the larger astroalign workflow). So the purpose of this issue is to feel out the astroalign team (which I think means @martinberoiz, @leliel12, and @BrunoSanchez) on the idea of moving some of this functionality over to Astropy.

To be more concrete, the idea here would be to take find_transform (and the related private functions it requires) and move them to astropy.coordinates. There would probably need to be some minor changes to the interface to fit it into relevant Astropy machinery, including possibly making a few of the currently-private functions part of a slightly more flexible user-facing API. But I've already had a quick look and I think it's not much effort - I could do that myself unless someone else wants to. The rest of astroalign's functionality (i.e. the more convenient quick-and-easy interfaces and apply_transform function) would remain in astroalign since that provides a nice one-stop-shop for people just trying to solve the higher-level task that don't care about the details of the matching.

If that happens, ideally astroalign would be updated to use the Astropy implementation. The idea being that way we won't end up getting divergent code bases and we can work together to fix bugs, etc. The one downside of that approach from astroalign's perspective is it would require bringing back an astropy dependency, which I gather from #34 and #22 is potentially a concern. I think it's a bigger win to make this functionality part of the astropy core (and thereby usable in Astropy itself and more easily in other affiliated/coordinated packages), but of course I would 😉. So hence bringing the topic up before doing anything more.

Note I'm also concious of the "credit" question. To do the plan I'm proposing I would try to extract the git history so that all of the commits from astroalign that touches those parts of the code would be preserved. I think it would also make sense to request in the docs that anyone using the astropy function should also try to cite the astroalign paper (at least, if that's what the astroalign team desires).

So what do y'all think?

[martinberoiz]

Hi Erik,

I spoke with the rest of the team, and we think it'd be great if we can move find_transform over to astropy's core.
I meant to contact other astronomy packages before as well, but I got caught with work in other projects. Merging it this way will be easier for others to use the feature.

There are a few things to iron-out, I think.

Do you want to keep the functionality of aligning using images (numpy arrays), or will it be enough to do it with lists of (x, y) source positions?
Both ways are possible now, but if you want to keep working with images, then source extraction will be important.
I'm thinking that you may want to use photutils for source detection, unless you want to add sep to astropy's dependencies.
I have a local branch on my computer where I tried to work with photutils, but due to my lack of familiarity with it, I couldn't make it pass the tests, so I never really did that switch.

Also, full disclosure, find_transform returns two things:

• T, the transformation between the images, is pretty solid and returns the correct transformation most of the time.
• But the tuple (source_pos_array, target_pos_array) has a known issue (Multiple partners for list of stars #41), in which one source may be falsely associated to a few others nearby. Usually this second tuple is not needed, though, and you may want to get rid of it in the API.

BTW it's alright if you need to change the public or private API to fit astropy's way of doing things.

Regarding integration with astropy, I was at first reluctant to add it as a dependency, since IMHO it seemed like an overkill just to use a couple of classes here and there.
Recently though, we were experimenting with @moemyself3 to add a rough WCS estimate based on an approximate knowledge of the field center and FOV. In that scenario we would definitely have to add astropy as a dependency and I feel it's justified.

So, overall, I think it would be no problem if the feature is "uploaded" to astropy's core and then we import it from there to use it. We will both benefit from it.

And finally, regarding credit, your suggestions seem fair and if you can do that thing with git's history, it'd be great (I didn't know you could extract commits like that tbh).

Thanks for contacting us and let us know here if you need more info or help.

[astrofrog]

I was very interested to read the discussion in this issue, and I think it would be great to move find_transform over to astropy core. I was curious if this is something the astroalign team would be interested in doing, or whether you would like one of the astropy core developers to help. I may have some availability in the coming year and if it is something you would be happy for someone else to do, this might be something I or others can apply for funding for from astropy to do (or if you would like to do it yourselves, you could also apply for funding from astropy)

[martinberoiz]

Hi Thomas,

Yes, I'd be happy if we can integrate this into astropy. If you have the time availability to do this, then that'd be great. I'm spread very thin already and I barely have the time to keep up with the issues here. I could probably help but I don't think I can dedicate long hours unfortunately.

Let me tell you the status of astroalign, I have a couple of merges coming. One is to update numpy's new interface of random (this doesn't affect much of the code, mainly just the tests). The other merge deals with masking out regions of the image. After those merges, I don't expect any other major change to the code.

If we were to integrate with astropy, there are a few issues that you need to consider. I think they are all solvable. find_transform depends mainly on scikit-image's warp method, that I understand now was moved to a C function. It also depends on scipy for a KD-tree data structure, this is probably easily replaceable. And it also depends on sep, which could be replaced with something from astropy.

I think those are the major hurdles to tackle for the migration. I don't think any of them is a true block, but it will require considerable work, I'm afraid.