Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: indexing a QTable with a Quantity column doesn't work as expected #16036

Open
neutrinoceros opened this issue Feb 14, 2024 · 4 comments
Open

Comments

@neutrinoceros
Copy link
Contributor

neutrinoceros commented Feb 14, 2024

import astropy.units as u
from astropy.table import QTable

t = QTable({"a": u.Quantity([0], unit="m")})
t.add_index(["a"])
t.loc[t["a"][0]]

fails with

Traceback (most recent call last):
  File "/Users/clm/dev/astropy-project/coordinated/astropy/t.py", line 6, in <module>
    t.loc[t["a"][0]]
    ~~~~~^^^^^^^^^^^
  File "/Users/clm/dev/astropy-project/coordinated/astropy/astropy/table/index.py", line 888, in __getitem__
    rows = self._get_rows(item)
           ^^^^^^^^^^^^^^^^^^^^
  File "/Users/clm/dev/astropy-project/coordinated/astropy/astropy/table/index.py", line 867, in _get_rows
    for key in item:
  File "/Users/clm/dev/astropy-project/coordinated/astropy/astropy/units/quantity.py", line 1283, in __iter__
    raise TypeError(
TypeError: 'Quantity' object with a scalar value is not iterable

In contrast, the following works

import astropy.units as u
from astropy.table import QTable

t = QTable({"a": u.Quantity([0], unit="m")})
t.add_index(["a"])
t.loc[t["a"][0].value]  # passing `.value` instead of the full quantity

Using a Table instead of a QTable also works as expected

import astropy.units as u
from astropy.table import Table

t = Table({"a": u.Quantity([0], unit="m")})
t.add_index(["a"])
t.loc[t["a"][0]]

I'm not 100% sure the current behaviour is actually buggy: this is a matter of intent.
This was first discussed in #15826 where @taldcroft found that multi-indexing wouldn't work seamlessly for Quantity indices, as he assumed.

This report is adapted from my comment #15826 (comment)_

@neutrinoceros neutrinoceros changed the title BUG: indexing a (Q)Table with a Quantity column doesn't work as expected BUG: indexing a QTable with a Quantity column doesn't work as expected Feb 14, 2024
@mhvk
Copy link
Contributor

mhvk commented Feb 14, 2024

Note that for non-scalar input, the error is different,

t = QTable({"a": u.Quantity([20., 21., 22.], unit="m")})
t.add_index(["a"])
t.loc[t["a"][1:]]
# KeyError: 'No matches found for key 21.0 m'

@mhvk
Copy link
Contributor

mhvk commented Feb 14, 2024

Actually, the cause is easy (though maybe a nice fix less so): the index uses a regular Table to store the index columns, which means that Quantity get converted to Column instances, and therefore the look-up doesn't work: see

elif len(columns) == 1:
col = columns[0]
row_index = Column(col.argsort(kind="stable"))
data = Table([col[row_index]])

If I change Table to QTable there, the example with a slice works fine (note there is another explicit Table on l.113)

The scalar still does not work, but can also be made to work by changing

if not isinstance(item, (list, np.ndarray)): # single element

to

            if not isiterable(item):  # single element

(with a from astropy.utils import isiterable on top).

For an implementation, possibly easiest is not to pass in columns to Index but rather a Table/QTable with the desired columns (since the columns are going to be added to a table anyway).

@neutrinoceros
Copy link
Contributor Author

For an implementation, possibly easiest is not to pass in columns to Index but rather a Table/QTable with the desired columns (since the columns are going to be added to a table anyway).

I think it's feasible but it isn't as easy as I first thought. I'm game to try it out again if no objection is raised !

@mhvk
Copy link
Contributor

mhvk commented Feb 15, 2024

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants