Support Table fuzzy or cross-match join with SkyCoord, Quantity, Column #10169

taldcroft · 2020-04-18T22:53:08Z

Description

This PR adds the ability to specify a custom matching function for table joins. In particular this makes it possible to do cross-match table joins on SkyCoord, Quantity, or standard columns, where column entries within a specified distance are considered to be matched.

The change to join is to add a new kwarg join_funcs that is a dict of functions which are responsible for doing the actual cross-matching for a key column.

With sufficient ingenuity one could doing fuzzy text joins by supplying an appropriate join function.

To do:

Narrative docs describing using this and also how to write your own join_func.
Add join_funcs to join docstring.
Check for issues this might close.

taldcroft · 2020-04-28T10:48:58Z

@mhvk - ping to make sure this is on your radar for 4.1.

mhvk · 2020-04-28T13:17:48Z

Definitely on the radar, but trying to get #10210 wrapped up first... Unfortuantely, timing of feature freeze is bad in Spring, with lots of student projects/committee meetings.

mhvk

This is really neat! Until I saw the join code, I hadn't actually quite appreciated it was also a quite tricky problem generally (though obviously so thinking about it), so those helper functions are going to be very useful as inspiration for others.

As you can see, I only have two comment, which are quite trivial. This makes me worry a bit, especially as it is quite late and hence I'm a bit sleepy, so maybe I can ping @adrn to have a look as well (as someone who might well want to use this...).

mhvk · 2020-04-29T01:20:09Z

astropy/table/operations.py

+    ----------
+    distance : Quantity (angle or length)
+        Maximum distance between points to be considered a join match
+    distance_kind : str


In principle, this could be inferred based on the unit of distance.

~~Good idea, will do.~~

Even then I much prefer one less kwarg (simple is better than complex), on further reflection I think this "guessing" may be ambiguous because of u.dimensionless_angles(). Because of this equivalency one can pass in a dimensionless Quantity and it could either be a 3d cartesian distance or a dimensionless angular distance. I don't know of any rock solid way to disambiguate, and even if we come up with one it will be complicated to explain to users. So I think it might be best to just leave this be (explicit is better than implicit).

Can dimensionless ever be a 3-d distance? I guess the test would be ...unit.is_equivalent(u.radian).

But perhaps an alternative is to have the default be determined by the unit? It is good to have the user be able to pass in an explicit distance_kind so that things will simply fail if distance has the wrong units.

mhvk · 2020-04-29T01:32:14Z

astropy/table/operations.py

+    return join_func
+
+
+def join_distance(distance, kdtree_args=None, query_args=None):


Might it make sense to re-purpose people's experience with isclose and have an atol and rtol? (in which case this might become join_close).

This would need to be a completely new function since this is explicitly applicable to N-dimensional points and uses KDTree to find the neighbors. For N-dim there is no concept of rtol.

For the 1-d case, I actually don't know how to do a cross match with atol and rtol without just doing the N^2 cross-match.

There is another problem I thought of, which is that this whole concept is based on the cross-matching being symmetric. If point A is "close" to point B, then it is required that B is "close" to A. That is true for a distance (atol), but breaks down for rtol.

In [7]: np.isclose(1, 0, atol=0.8, rtol=0.8) Out[7]: False In [8]: np.isclose(0, 1, atol=0.8, rtol=0.8) Out[8]: True

Oh, yes, my brain was not quite there with the 2-d case. Let's just stick with distance which is nicely defined. (and, yes, isclose is not symmetric unlike the math equivalent; numpy/numpy#10161)

mhvk · 2020-04-29T15:06:16Z

circle-ci failures try to import scipy...

taldcroft · 2020-04-29T15:31:47Z

circle-ci failures try to import scipy...

Ah right, so need to skip test if no scipy.

adrn

Overall, looks great, and yes this will make writing simple cross-matching code much faster!

adrn · 2020-04-30T12:59:32Z

astropy/table/operations.py

@@ -87,9 +88,238 @@ def _get_out_class(objs):
    return out_class


+def join_skycoord(distance, distance_kind='sky'):


I'm usually against arguments that allow string values - why not require passing in the distance function directly, e.g.,

Suggested change

def join_skycoord(distance, distance_kind='sky'):

def join_skycoord(distance, distance_func=search_around_sky):

(some code below can be simplified)

I see your point, but passing in the function is harder for the user (extra import required). We have had this philosophical difference going back to forever, but in the end we agreed that passing in strings should generally be allowed instead of requiring imported objects.

How about meeting in the middle and allowing a string which must correspond to a function in coordinatesi.e. (try getattr(astropy.coordinates, distance_func), or a function that has the same API as the current search_around functions. So instead 'sky' or '3d' it would be the full name.

And I'll say that I am usually against putting in functionality where we have not yet identified a realistic use case. In this case allowing passing in a user-defined distance function that meets the search_around API exactly. So a plan B is to not allow passing in a function now and wait for user to request that feature. And in fact in that case the user could make their own join_func.

Yea, philosophy. For completeness: This doesn't require another import if import astropy.coordinates as coord is already done (pass coord.search_around_sky). And the (weak) argument against string values is that it requires reading to see what values are allowed - if the default was coord.search_around_sky, the user might think to try coord.search_<TAB> (in a notebook) to see what other options there are. But, yes, this is a weak argument and perhaps too "behavioral"...

In practice, though, I don't feel very strongly about this, so I leave it up to you!

Yes, in my world I just do from astropy.coordinates import SkyCoord and that is all I need! 😄 And in practice time is running out ... I wasn't totally happy with distance_kind and I like distance_func better.

Understood - thanks for your work on this!

Also, if the default of the function is based on the unit, this will very rarely be needed!

docs/table/operations.rst

taldcroft added table coordinates whatsnew-needed labels Apr 18, 2020

taldcroft added this to the v4.1 milestone Apr 18, 2020

taldcroft force-pushed the table-fuzzy-join branch 2 times, most recently from 26ab275 to b97033e Compare April 24, 2020 15:15

taldcroft requested a review from mhvk April 26, 2020 10:22

taldcroft force-pushed the table-fuzzy-join branch from b97033e to 070a02e Compare April 26, 2020 11:02

mhvk reviewed Apr 29, 2020

View reviewed changes

adrn reviewed Apr 30, 2020

View reviewed changes

taldcroft force-pushed the table-fuzzy-join branch from 8e7ed6f to 7b7bb9f Compare May 1, 2020 21:52

taldcroft added 13 commits May 2, 2020 09:31

Initial support for join_funcs inluding skycoord_join

3da44c4

Add docs and distance_join() function

aaa6cf3

More docs and minor code tweaks

b7942fd

Add tests and more docs updates

c9ad0a5

Update changelog

c846078

More tests and docs

7b80694

Change join func names + docs fixes

7eca832

Add a test of multiple join keys and other columns

8ad434f

Remove outdated OLDER_SCIPY 0.12 skipif

d68bf9b

Skip tests unless scipy is installed

2a46239

Documentation tweaks

6433a45

Require scipy for fuzzy join docs

a7945a3

Change join_skycoord arg to distance_func and allow func

ce76805

taldcroft force-pushed the table-fuzzy-join branch from 7b7bb9f to ce76805 Compare May 2, 2020 13:34

taldcroft merged commit 9ad423b into astropy:master May 2, 2020

taldcroft deleted the table-fuzzy-join branch May 2, 2020 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Table fuzzy or cross-match join with SkyCoord, Quantity, Column #10169

Support Table fuzzy or cross-match join with SkyCoord, Quantity, Column #10169

taldcroft commented Apr 18, 2020 •

edited

taldcroft commented Apr 28, 2020

mhvk commented Apr 28, 2020

mhvk left a comment

mhvk Apr 29, 2020

taldcroft Apr 29, 2020 •

edited

taldcroft Apr 29, 2020

mhvk Apr 29, 2020

mhvk Apr 29, 2020

taldcroft Apr 29, 2020

mhvk Apr 29, 2020

mhvk commented Apr 29, 2020

taldcroft commented Apr 29, 2020

adrn left a comment

adrn Apr 30, 2020

taldcroft Apr 30, 2020

taldcroft Apr 30, 2020

adrn Apr 30, 2020

taldcroft Apr 30, 2020

adrn Apr 30, 2020

mhvk Apr 30, 2020

		return join_func


		def join_distance(distance, kdtree_args=None, query_args=None):

		@@ -87,9 +88,238 @@ def _get_out_class(objs):
		return out_class


		def join_skycoord(distance, distance_kind='sky'):

	def join_skycoord(distance, distance_kind='sky'):
	def join_skycoord(distance, distance_func=search_around_sky):

Support Table fuzzy or cross-match join with SkyCoord, Quantity, Column #10169

Support Table fuzzy or cross-match join with SkyCoord, Quantity, Column #10169

Conversation

taldcroft commented Apr 18, 2020 • edited

Description

taldcroft commented Apr 28, 2020

mhvk commented Apr 28, 2020

mhvk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taldcroft Apr 29, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhvk commented Apr 29, 2020

taldcroft commented Apr 29, 2020

adrn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taldcroft commented Apr 18, 2020 •

edited

taldcroft Apr 29, 2020 •

edited