JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
9 stars 4 forks source link

Merge all OSW converters #1455

Closed smaticka closed 8 months ago

smaticka commented 8 months ago

Description

Currently a separate converter exists for each CYGNSS, Muon, and Spire data. This PR is to merge the 3. (and then remove the originals).

Example of how to run: python3 $JEDI_SRC/iodaconv/src/hdf5/osw_2ioda.py -i filename_input.nc -o filename_output.nc

Issue(s) addressed

Resolves #1452

Checklist

smaticka commented 8 months ago

the converter produces the same results as the original 3 converters, aside for the global attribute labeling and the unused variables in the Muon converter.

the 3 original converters need to be deleted in this PR as well, but are left in for now in case people want them for testing/comparison.

BenjaminRuston commented 8 months ago

@smaticka was able to run this for muon data again after my changes please put it through the paces but think this is pretty close, we will want to remove: {spire,muon,cygnss}_2ioda.py files as part of this PR as well

smaticka commented 8 months ago

@BenjaminRuston I had to change the way you were appending strings, but now it works for me. I tested on the cygnss data.

BenjaminRuston commented 8 months ago

@srherbener think this is ready to go now. Do have a question for you. Try to use the IODA missing values for int and float but wondering is there one for string as well see:

https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/hdf5/osw_2ioda.py#L68

also more trivial bits, but would maybe like to consolodate these too into the pyiodaconv/def_jedi_utils: https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/hdf5/osw_2ioda.py#L75

can we use these ioda_int_type rather than np.int32 ? https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/pyiodaconv/def_jedi_utils.py#L23

srherbener commented 8 months ago

@srherbener think this is ready to go now. Do have a question for you. Try to use the IODA missing values for int and float but wondering is there one for string as well see:

https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/hdf5/osw_2ioda.py#L68

also more trivial bits, but would maybe like to consolodate these too into the pyiodaconv/def_jedi_utils:

https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/hdf5/osw_2ioda.py#L75

can we use these ioda_int_type rather than np.int32 ?

https://github.com/JCSDA-internal/ioda-converters/blob/33289418f9eb2b64364d884adda0a9ae652bfb6b/src/pyiodaconv/def_jedi_utils.py#L23

I think we should use numpy data types when the converter is numpy array based, and python data types when the converter is python object based (eg, lists).

BenjaminRuston commented 8 months ago

@rmclaren and @CoryMartin-NOAA just for reference this was merged after the PR: https://github.com/JCSDA-internal/ioda-converters/pull/1454

and passed the tests as well,,