Check for nan dateobs before converting to Time

astropy / pyvo

An Astropy affiliated package providing access to remote data and services of the Virtual Observatory (VO) using Python.

https://pyvo.readthedocs.io/en/latest

BSD 3-Clause "New" or "Revised" License

74 stars 50 forks source link

Check for nan dateobs before converting to Time #550

Closed duytnguyendtn closed 1 month ago

duytnguyendtn commented 1 month ago

This small PR fixes #549 by adding an additional check on the ucd value of VOX:Image_MJDateObs before trying to construct an astropy.Time object. Bug found in SIA, though this PR ALSO preemptively performs the same check for SSA's ssa:DataID.Date.

Changelog will be added immediately after this PR is published, and a PR number is assigned

duytnguyendtn commented 1 month ago

Just saw @ManonMarchand tagged this to the 1.5.2 bugfix milestone. Didn't see an entry for this version, so I moved my changelog entry to a new 1.5.2 header. Feel free to edit if I didn't quite get it right!

ManonMarchand commented 1 month ago

Awesome, thanks!

Would you be comfortable adding a test? A suggestion on how to proceed could be :

edit test/data/sia/dataset.xml to add a second record with a nan
add a test on _test_result in the sia tests

And it would also be nice to rebase the commits on the changlelog to group them together. If you need help with this don't hesitate to ask.