609 imap hi l1c pointing set cdf #627
Conversation
parameterize hi cli tests so that all level1 is tested in a single function add CdfAttributeManager to hi l1c processing
Update cli test for Hi to include coverage for mocked function signature
add unit test coverage for hi_l1c functions
subagonsouth
requested review from
a team,
bourque,
sdhoyt,
greglucas,
tech3371,
maxinelasp,
daralynnrhode and
anamanica
and removed request for
a team
subagonsouth
added
Ins: Hi
| # SPDF says this should be the center of the time bin, but instrument | ||
| # teams may disagree. |
There was a problem hiding this comment.
For some reason I thought you could specify the left/center/right accumulation bin, but today I learned!
For ISTP the time value of a record refers to the center of the accumulation period for the record if the measurement is not an instantaneous one.
There was a problem hiding this comment.
Thanks for grabbing that text. I should have put that into the TODO originally. I will add it in for clarity.
| ) | ||
|
|
||
| # Generate label variables | ||
| data_vars["esa_step_label"] = xr.DataArray( |
There was a problem hiding this comment.
Nice, so these labels do work as data variables? We should make a ticket to update the other instruments later.
There was a problem hiding this comment.
SKTEditor seemed fine with them being variables rather than coordinates. I wrote up ticket #628 to capture this.
| return dataset | ||
|
|
||
|
|
||
| def full_dataarray(name, attrs, coords: dict, shape=None): |
There was a problem hiding this comment.
This is a really nice helper! For later: I think we should look at moving this out more generally and using it across instruments if possible.
| @@ -107,56 +107,37 @@ def test_codice(mock_codice_l1a, mock_instrument_dependencies): | |||
| assert mocks["mock_upload"].call_count == 1 | |||
|
|
|||
|
|
|||
| @mock.patch("imap_processing.cli.hi_l1a.hi_l1a") | |||
| def test_hi_l1a(mock_hi_l1a, mock_instrument_dependencies): | |||
| @pytest.mark.parametrize("data_level, n_prods", [("l1a", 2), ("l1b", 1), ("l1c", 1)]) | |||
There was a problem hiding this comment.
Nice use of parametrization here! I've been wondering if we could parameterize over some things in this file with all the duplication.
subagonsouth
merged commit 33a950a
into
IMAP-Science-Operations-Center:dev
| DISPLAY_TYPE: no_plot | ||
| FORMAT: F5.2 | ||
| LABLAXIS: Latitude |
| UNITS: deg | ||
| VALIDMIN: 0 | ||
| VALIDMAX: 180 |
There was a problem hiding this comment.
If we are starting at 0, would max be 179? I don't know for sure though.
| UNITS: deg | ||
| VALIDMIN: 0 | ||
| VALIDMAX: 360 |
There was a problem hiding this comment.
same here if above needs update
| LABL_PTR_1: esa_step_label | ||
| LABL_PTR_2: spin_bin_label | ||
| DISPLAY_TYPE: stack_plot |
There was a problem hiding this comment.
cool. Didn't know about this
| cdf_manager = CdfAttributeManager(imap_module_directory / "cdf" / "config") | ||
| cdf_manager.load_global_attributes("imap_hi_global_cdf_attrs.yaml") | ||
| cdf_manager.load_variable_attributes("imap_hi_variable_attrs.yaml") |
There was a problem hiding this comment.
This is ok for now. After SIT-3, we will need to update this to this. I believe this does minor validation and sounds like we may expand this afterwards.
# create the attribute manager for this data level
attr_mgr = ImapCdfAttributes()
attr_mgr.add_instrument_global_attrs(instrument="lo")
attr_mgr.add_instrument_variable_attrs(instrument="lo", level="l1a")
attr_mgr.add_global_attribute("Data_version", data_version)
Change Summary
Adds Hi l1c processing for pset data
Overview
Adds generation of empty l1c pset product to Hi processing.
New Dependencies
None
New Files
Deleted Files
None
Updated Files
Testing