You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Support vstack and similar operations for SkyCoord #6348
I use SkyCoord as a mixin quite a bit in tables - it's very common to have catalogs with RA/Dec coordinates as one of the axes, and of course it's convenient to just drop SkyCoord's directly into that.
However, if I try to vstack two tables with SkyCoords as one of the columns, I get the dreaded:
NotImplementedError: vstack unavailable for mixin column type(s): SkyCoord
This seems like it should be straightforward given that SkyCoords pretty much behave like table columns... Of course I'm sure it's not that simple... but it's a pretty important use case, and maybe it's time has come?
@eteq - completely agree. The generic way of supporting mixin operations like join, hstack and vstack requires that the object be mutable because the code first creates an empty instance of the object with the right shape and attributes, then fills in the elements appropriately. See e.g. the code here for vstack. It makes a new column based on the list of columns to be stacked using the Info.new_like() method. This is asking the mixin class to do all the meta merging and figure out if the inputs can really be stacked and what the output should look like. Then it sets the output via slice or fancy indexing array assignment operations.
We need to set items in SkyCoord in this context. So this is the time to discuss making SkyCoord mutable, which I know you have a strong opinion about. Note that in theory the vstack code could do this slice setting with a context manager that temporarily enables mutability.
first creates an empty instance of the object with the right shape and attributes, then fills in the elements appropriately.
Oh, I didn't realize that @taldcroft ... that explains why this is harder. I guess I don't understand why this is a necessity, though? That is, why can't it instead (optionally) create the correct thing initially and drop it in? That would be a lot easier to implement as it stands...
Note that in theory the vstack code could do this slice setting with a context manager that temporarily enables mutability.
Hmm, perhaps I could get behind that? Perhaps a topic for elsewhere, though, depending on the result of the above.
That is, why can't it instead (optionally) create the correct thing initially and drop it in?
I assume that you mean something like the new_like() method would create the new stacked SkyCoord column from the input columns by first doing the vstacking on the individual data attributes (e.g. ra, dec, distance) and then instantiating with the vstack'ed inputs? If so...
The problem is that this model does not generalize well to the case of missing values. In other words, there is no guarantee when you stack two tables that all the columns match, so the new column may be longer than the sum of the length of the inputs. Likewise for hstack you don't know that they are all the same length, and for join you don't know that there won't be missing rows in the join.
So this makes things ugly. There is currently the table high level operation code (for vstack, hstack, join) which knows all the details for assembling the output for any case. You would then need to move that functionality to be specific to each class that can be an input, effectively having vstack, hstack, and join functionality for every supported table mixin class. This is certainly possible but I would argue it is not preferred.
Instead if you stipulate that an object has a setitem that supports slice and fancy index setting, then all the specifics of vstack, hstack, and join stay in the table code where they belong. The only thing specific to the class is new_like, where they class knows how to construct an empty object of the right length given the input columns. But it doesn't have to know what is going in to it.
@eteq@taldcroft - Would the use case of matching two tables fall under this issue, too? I mean both have a SkyCoord column, but with some non zero matching separation. Would joining the two of those tables be possible when this gets implemented, or will require a convenience function (with probably a seapration argument)?
Would the use case of matching two tables fall under this issue, too? I mean both have a SkyCoord column
@bsipocz - yes, sort of. You would need to do pre-processing of the table to do the matching in advance and have something like an integer id column that then allows join-style matching. A fundamental part of the table join algorithm is being able to sort on the match key, so a SkyCoord won't work.
However, if you did make the id columns and then used those as the key column, then this functionality (along with what I have argued is a pre-requisite, #6394) would do the trick.
Activity
taldcroft commentedon Jul 7, 2017
@eteq - completely agree. The generic way of supporting mixin operations like join, hstack and vstack requires that the object be mutable because the code first creates an empty instance of the object with the right shape and attributes, then fills in the elements appropriately. See e.g. the code here for vstack. It makes a new column based on the list of columns to be stacked using the
Info.new_like()method. This is asking the mixin class to do all the meta merging and figure out if the inputs can really be stacked and what the output should look like. Then it sets the output via slice or fancy indexing array assignment operations.We need to set items in SkyCoord in this context. So this is the time to discuss making SkyCoord mutable, which I know you have a strong opinion about. Note that in theory the vstack code could do this slice setting with a context manager that temporarily enables mutability.
eteq commentedon Jul 25, 2017
Oh, I didn't realize that @taldcroft ... that explains why this is harder. I guess I don't understand why this is a necessity, though? That is, why can't it instead (optionally) create the correct thing initially and drop it in? That would be a lot easier to implement as it stands...
Hmm, perhaps I could get behind that? Perhaps a topic for elsewhere, though, depending on the result of the above.
taldcroft commentedon Jul 26, 2017
I assume that you mean something like the
new_like()method would create the new stacked SkyCoord column from the input columns by first doing the vstacking on the individual data attributes (e.g. ra, dec, distance) and then instantiating with the vstack'ed inputs? If so...The problem is that this model does not generalize well to the case of missing values. In other words, there is no guarantee when you stack two tables that all the columns match, so the new column may be longer than the sum of the length of the inputs. Likewise for hstack you don't know that they are all the same length, and for join you don't know that there won't be missing rows in the join.
So this makes things ugly. There is currently the table high level operation code (for vstack, hstack, join) which knows all the details for assembling the output for any case. You would then need to move that functionality to be specific to each class that can be an input, effectively having vstack, hstack, and join functionality for every supported table mixin class. This is certainly possible but I would argue it is not preferred.
Instead if you stipulate that an object has a setitem that supports slice and fancy index setting, then all the specifics of vstack, hstack, and join stay in the table code where they belong. The only thing specific to the class is
new_like, where they class knows how to construct an empty object of the right length given the input columns. But it doesn't have to know what is going in to it.bsipocz commentedon Nov 30, 2017
@eteq @taldcroft - Would the use case of matching two tables fall under this issue, too? I mean both have a SkyCoord column, but with some non zero matching separation. Would joining the two of those tables be possible when this gets implemented, or will require a convenience function (with probably a seapration argument)?
taldcroft commentedon Nov 30, 2017
@bsipocz - yes, sort of. You would need to do pre-processing of the table to do the matching in advance and have something like an integer
idcolumn that then allows join-style matching. A fundamental part of the table join algorithm is being able to sort on the match key, so a SkyCoord won't work.However, if you did make the
idcolumns and then used those as thekeycolumn, then this functionality (along with what I have argued is a pre-requisite, #6394) would do the trick.mhvk commentedon Dec 22, 2017
Removing milestone, since we're not going to make this.