Update Default Attributes, Overwrite Policy, and Attribute Validation #100
Conversation
Codecov Report
@@ Coverage Diff @@
## main #100 +/- ##
==========================================
- Coverage 94.49% 94.47% -0.02%
==========================================
Files 10 10
Lines 1707 1756 +49
==========================================
+ Hits 1613 1659 +46
- Misses 94 97 +3
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
|
|
||
| def test_min_max_Epoch16(): | ||
| """Get min/max values for Epoch16 types""" | ||
| minval, maxval = HermesDataSchema()._get_minmax(const.CDF_EPOCH16) | ||
| # Make sure the minimum isn't just plain invalid | ||
| assert minval == datetime.datetime(1, 1, 1) | ||
| assert maxval == datetime.datetime(9999, 12, 31, 23, 59, 59) | ||
| assert minval == datetime.datetime(1900, 1, 1, 0, 0, 0, 0) |
There was a problem hiding this comment.
Isn't this 01-Jan-0000 00:00:00.000.000.000.000?
There was a problem hiding this comment.
There was a problem hiding this comment.
The same is true for epoch 8 bit.
There was a problem hiding this comment.
The table on Page 57 looks like its referring to the FILLVAL attribute rather than the VALIDMIN/MAX attributes. From a brief skim through the PDF it does not look like there are recommendations for VALIDMIN/MAX default values.
I updated these here because we were getting overflow errors when loading and saving CDF files so I updated them to more reasonable MIN and MAX values.
There was a problem hiding this comment.
Sorry i misunderstood the spec.
| @@ -306,24 +306,24 @@ def test_min_max_TT2000(): | |||
| """Get min/max values for TT2000 types""" | |||
| minval, maxval = HermesDataSchema()._get_minmax(const.CDF_TIME_TT2000) | |||
| # Make sure the minimum isn't just plain invalid | |||
| assert minval == datetime.datetime(1, 1, 1) | |||
| assert maxval == datetime.datetime(2292, 4, 11, 11, 46, 7, 670776) | |||
| assert minval == datetime.datetime(1900, 1, 1, 0, 0, 0, 0) | |||
There was a problem hiding this comment.
Shouldn't we be using astropy time objects here? since we are supposed to be using them everywhere?
There was a problem hiding this comment.
Spacepy loads these attributes as datetime objects when loading a CDF file. So if we want to make everything astropy time everywhere, we would need to create wrappers around the spacepy attribute loading as well as update these derivation functions. If we want to go that route that might warrant a separate issue and PR.
There was a problem hiding this comment.
Grrrr. spacepy does support astropy time but does not use it "natively".
closes #98
closes #99