search

MillionConcepts / pdr

[P]lanetary [D]ata [R]eader - A single function to read all Planetary Data System (PDS) data into Python
Other
62 stars 7 forks source link

LRO bsp pointers not yet fully implemented #69

Closed Sierra-MC closed 2 months ago

Sierra-MC commented 2 months ago

From @gaelccc review

I tried another format mentioned in the example, but officially supported (as per the documentation). Notably bsp files from the LRO repository. In this case the method .read() works, but doesn't seem to produce useful output. All I get is the label of the file and a warning stating that the bsp pointer is still not fully implemented.

joss-review

Sierra-MC commented 2 months ago

@gaelccc could you give an example of the bsp files you're referring to?

If these are the files you're referring to: https://naif.jpl.nasa.gov/pub/naif/pds/data/lro-l-spice-6-v1.0/lrosp_1000/data/spk/

Those are spice files and aren't supported by pdr. There is a separate package spiceypy that handles spice files (actually authored by our other reviewer Andrew Annex). If these are what you're referring to it's a good catch that we don't directly state we don't support spice kernels and I'll add that to our documentation.

cmillion commented 2 months ago

@gaelccc I am also not able to see where LRO BSP files are referenced in the example nor listed as officially supported. If either of those things is happening, then it is in error. Can you please point me to what I'm missing? Thank you.

gaelccc commented 2 months ago

Apologies! While copy/pasting my comments I omitted a fundamental "not":

I tried another format not mentioned in the example, but officially supported (as per the documentation)

Yes, I am referring to BSP/SPK files. I wasn't expecting pdr to support them, but since the supported datasets document states in few instances trajectory data (e.g., Galileo), I just naively assumed it referred to BSP.

Now after a deeper look I see that the trajectory data appears only on specific missions, in which case I assume the archivers decided to use a different format and not BSP, that is the one you are referring to.

I think this issue can easily be closed by just specifying somewhere something like: trajectory data (no BSP/SPK)

Sierra-MC commented 2 months ago

Okay, I've updated the supported_datasets file to add some information on spice files in this commit.