raise error if no valid timebase specified #139
Conversation
Codecov ReportBase: 77.49% // Head: 78.31% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #139 +/- ##
==========================================
+ Coverage 77.49% 78.31% +0.81%
==========================================
Files 7 7
Lines 1271 1282 +11
==========================================
+ Hits 985 1004 +19
+ Misses 286 278 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| indctd = np.where(~np.isnan(sensor.NAV_DEPTH))[0] | ||
| # If present, use the timebase specified in ncvar: timebase in the | ||
| # deployment yaml. | ||
| if 'timebase' not in ncvar: |
There was a problem hiding this comment.
We can probably make a little test for this behaviour by loading a file, and removing 'timebase' from the parsing of it, and checking for this error. I know that seems a little contrived, but testing behaviour is a way of documenting it so someone doesn't see this, and wonder why we don't have a fallback ;-)
| if 'timebase' not in ncvar: | ||
| raise ValueError("Must specify timebase:source in netcdf_variables section of deployment yaml") | ||
| if ncvar['timebase']['source'] not in sensor.columns: | ||
| raise ValueError(f"timebase source: {ncvar['timebase']['source']} not found in pld1 columns") |
There was a problem hiding this comment.
Same here we could load a parquet file, remove the sensor and trip the error?
|
This commit adds tests for a timebase that doesn't match any of the variables and for if the timebase has been dropped from the yaml. In both cases, we check the error string for the expected error message. |
|
@jklymak is this ready to merge with the added test? |
|
Note I think that we should force timebase for slocums as well. |
alternative solution to #137. Forces user to specify a timebase