TOMToolkit / tom_base

The base Django project for a Target and Observation Manager
https://tom-toolkit.readthedocs.io
GNU General Public License v3.0
26 stars 46 forks source link

Add customizable class for formatting targets and datums into hermes … #1068

Closed jnation3406 closed 1 month ago

jnation3406 commented 1 month ago

…format

So the current problem is that sharing with HERMES requires certain fields for targets/photometry/spectroscopy. We have a base implementation to pull those fields out of what keys we think they might be under, but everyone's TOM is different and some people use different keys and other people (SNEX) use other related models.

So this PR adds a class that encapsulates the code that converts a Target or ReducedDatum model instance into the corresponding data dictionary in hermes format, so that this class can be extended by individual TOMs to match their data format. This is a bit more work for an individual TOM to configure if they need to, but it is flexible enough to support use cases like SNEX2 with their custom models for storing associated data.

jnation3406 commented 1 month ago

This looks great. I think there's some validation issue on the hermes end though? It won't accept data with just a limiting magnitude. When I try to submit such data I get '{"data":{"photometry":[{"brightness_unit":["This field may not be null."]}... as a response. Opening the data in Hermes results in no such error. Let me know if you'd like me to make an issue.

Hermes will set a default value if none is supplied for brightness_unit and limiting_brightness_unit, so I'll modify the code to not send that field if its null.

Also I should note that if there is no "filter" field in the ReducedDatum that will fail too, among other things. Cases like that are why a user should extend the HermesDataConverter to point to where that actually have that data, or hardcode it or whatever they want to share.