Lo L1C CDF Creation for SIT-3 #614
Conversation
sdhoyt
added
Ins: Lo
sdhoyt
requested review from
bourque,
greglucas,
subagonsouth and
tech3371
and removed request for
a team
| coords={ | ||
| "epoch": epoch_time, | ||
| "pointing_bins": pointing_bins, | ||
| "pointing_bins_label": pointing_bins_label, |
There was a problem hiding this comment.
I see that you are perhaps following what @tech3371 did. I didn't catch this in her PR, but I wonder if the label variables should really be coordinates or are they just regular variables?
There was a problem hiding this comment.
That's a good question. If they can be normal variables, I feel like it still makes more sense to have them as coordinates since they're basically string versions of the coordinates. I haven't looked at it much yet, but I'm hoping there is a good way to just convert the data to a string and use that as the label within write_cdf or cdflib instead of having to define attributes for the label since all the attributes are easily determined (ie. string length) or straight from the coordinate attributes.
| field = data_field.name.lower() | ||
| # Create a list of all the dimensions using the DEPEND_I keys in the | ||
| # YAML attributes | ||
| dims = [ |
There was a problem hiding this comment.
Nice. I like that you are extracting the dims form the attributes. I need to implement this in my pointing set code.
There was a problem hiding this comment.
I agree this is nice. I wonder if we should add this as a general helper to the AttributeManager class (not in this PR). get_variable_dims() (naming TBD) that would encapsulate this logic for everyone.
| assert dataset.mode.shape[0] == 1 | ||
| assert len(dataset.pivot_angle.shape) == 1 | ||
| assert dataset.pivot_angle.shape[0] == 1 | ||
| assert len(dataset.triples_counts.shape) == 3 |
There was a problem hiding this comment.
I think that the individual asserts for each variable can be done in a single line such as:
| assert len(dataset.triples_counts.shape) == 3 | |
| np.testing.assert_array_equal(dataset.triples_counts.shape, (1, 3600, 7)) |
There was a problem hiding this comment.
Maybe just compare against an array itself which will also check shape for you: np.testing.assert_array_equal(dataset.triples_counts, np.ones((1, 3600, 7)))
| pointing_bins_label: | ||
| CATDESC: Pointing bins | ||
| FIELDNAM: Pointing bins | ||
| FORMAT: A3 |
There was a problem hiding this comment.
I think that this has to be A4 to fit any value > 999.
| esa_step_label: | ||
| CATDESC: ESA Steps | ||
| FIELDNAM: ESA Steps | ||
| FORMAT: A3 |
There was a problem hiding this comment.
Can this just be A1 since esa_step will always be one character?
imap_processing/lo/l1c/lo_l1c.py
Outdated
| # The data type will be set in the data class when that's created | ||
| elif field == "exposure_time": | ||
| dataset[field] = xr.DataArray( | ||
| data=[7 * [np.float16(1)]], |
There was a problem hiding this comment.
I'd suggest using numpy array shapes for all of these temporary data fields which is a bit more explicit IMO.
| data=[7 * [np.float16(1)]], | |
| data=np.ones((1, 7), dtype=np.float16), |
There was a problem hiding this comment.
Yeah, I need to get better at recognizing when I can use numpy
| assert dataset.mode.shape[0] == 1 | ||
| assert len(dataset.pivot_angle.shape) == 1 | ||
| assert dataset.pivot_angle.shape[0] == 1 | ||
| assert len(dataset.triples_counts.shape) == 3 |
There was a problem hiding this comment.
Maybe just compare against an array itself which will also check shape for you: np.testing.assert_array_equal(dataset.triples_counts, np.ones((1, 3600, 7)))
imap_processing/tests/lo/test_cdfs/imap_lo_l1a_de_20100101_v001.cdf
Outdated
Show resolved
Hide resolved
Change Summary
Overview
This PR adds adds the following:
New Dependencies
None
New Files
Deleted Files
None
Updated Files
Testing