search

sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Change in kadi commands dtype #223

Closed javierggt closed 7 months ago

javierggt commented 2 years ago

Some dtypes in kadi commands changed in the latest version:

In [1]: from astropy.table import Table
   ...: import kadi
   ...: from kadi.commands import get_cmds
   ...: STARCAT_CMDS = Table(get_cmds('2003:001', type='MP_STARCAT'))['date', 'timeline_id']
   ...: print(kadi.__version__, get_cmds('2003:001', type='MP_STARCAT')['date'].dtype)
5.8.1 <U21

versus:

In [1]: from astropy.table import Table
   ...: import kadi
   ...: from kadi.commands import get_cmds
   ...: STARCAT_CMDS = Table(get_cmds('2003:001', type='MP_STARCAT'))['date', 'timeline_id']
   ...: print(kadi.__version__, get_cmds('2003:001', type='MP_STARCAT')['date'].dtype)
   ...: 
5.9.1 |S21

The one in 5.9.1 gives a table with bytes as opposed to unicode strings. That means that agasc supplement update will fail, because it used the mp_starcat_time strings as indices.

taldcroft commented 2 years ago

Discussed in slack, but for the record:

OK now I understand.. the V1 code get_cmds does actually call convert_bytestring_to_unicode on the CommandTable before returning it, but I got a bit too "clever" in the CommandTable class. There is code in the class to basically force the column dtypes to be always consistent and uniform (which should be a good thing), but it goes too far and doesn't allow the bytes columns to be turned to unicode.

Unfortunately there was no unit test that confirmed the expected dtypes for v1.

taldcroft commented 2 years ago

We are not fixing this. When we transition to v2 this can be closed.

taldcroft commented 7 months ago

We have transitioned to v2.