1147 hi l1c add calibration product coordinate to pset #1172
Conversation
subagonsouth
requested review from
vmartinez-cu,
laspsandoval and
maxinelasp
and removed request for
a team
| SCALE_TYP: linear | ||
| MONOTON: INCREASE | ||
| VALIDMIN: 0 | ||
| VALIDMAX: 4 |
There was a problem hiding this comment.
Could the max possibly change and if so does it make sense to have this be a larger value?
There was a problem hiding this comment.
I generally think of the valid max as a way to specify the range of expected values. Expanding this range just for a possible future change doesn't adhere to that definition IMO. I see that SPDF says VALIDMIN and VALIDMAX are the range of values expected for the lifetime of the mission. I see how that could be interpreted to allow for a future increase in the range.
I'm going to add a comment to the ticket that I have for defining the coincidence type to calibration product mapping to revisit this value. I will ask Paul about these details next time we meet and make appropriate changes in that ticket: #1170
| # TODO: define calibration product number to coincidence type mapping and | ||
| # use the number of calibration products here. I believe it will be 5 | ||
| # 0 for any, 1-4, for the number of detector hits. | ||
| n_calibration_prod = 5 |
There was a problem hiding this comment.
Instead of hardcoding 5, consider making n_calibration_prod a named constant
N_CALIBRATION_PRODUCTS = 5
There was a problem hiding this comment.
This value will be pulled from the mapping that is defined in #1170 so this hard coding here is just temporary.
| # use the number of calibration products here. I believe it will be 5 | ||
| # 0 for any, 1-4, for the number of detector hits. | ||
| n_calibration_prod = 5 | ||
| attrs = attr_mgr.get_variable_attributes( |
There was a problem hiding this comment.
Why is check_schema=False? I can't remember.
There was a problem hiding this comment.
SAMMI will strip off all of the dtype entries if I don't pass check_schema=False.
| CATDESC: Calibration product number | ||
| FIELDNAM: Coincidence types are combined into various calibration products |
There was a problem hiding this comment.
Should the CATDESC and FIELDNAM values be switched? I've been seeing longer descriptions being placed in the CATDESC field. FIELDNAM can be used to label a plot, so if that's the information that needs to be displayed, then switching these descriptions may not be necessary.
Change Summary
Overview
This adds the missing
calibration_proddimension to the pset data product.Updated Files
calibration_prodcoordinatecalibration_prod_labelmetadatacalibration_prodcoordinate toxr.Datasetcalibration_prod_labeldata_var toxr.Datasetcloses: #1147