sot / kadi

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

Fix numpy str FutureWarning and modernize get_ifot slightly #321

Closed taldcroft closed 4 months ago

taldcroft commented 4 months ago

Description

This fixes a FutureWarning caused by using np.str. The code got much cleaner due to an improvement in io.ascii since this kadi code was written. Now the converters kwarg can just be a dict of types (exactly the same as the types kwarg here). See https://docs.astropy.org/en/stable/io/ascii/read.html#parameters-for-read.

In addition I added a docstring and removed mutable values as keyword defaults in get_ifot.

Fixes #320

Interface impacts

The types dict option of get_ifot now requires that the type be specified as the actual type object (e.g. str or np.int64) instead of a str that is used in getattr(np, type).

I searched the sot repo on GitHub and found no instances outside of kadi that used the types kwarg of get_ifot.

Testing

Unit tests

kadi/commands/tests/test_commands.py ...................................................................................... [ 37%] kadi/commands/tests/test_states.py .......................x..............................................x................... [ 75%] .... [ 77%] kadi/commands/tests/test_validate.py .................... [ 86%] kadi/tests/test_events.py .......... [ 90%] kadi/tests/test_occweb.py ...................... [100%]



I don't have a `speedy` environment handy but that might be a good thing for the reviewer to test.

Independent check of unit tests by [REVIEWER NAME]
- [ ] [PLATFORM]:

### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
I ran the `kadi_update_events` script locally in current ska3-flight and confirmed that it ran successfully. I also confirmed that it fails without the fix changing `"str"` to `str` in `models.py`.