Repository navigation
Add new net_cdf4 codec - #288
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
f3dee39 to
b95f939
Compare
brenard-irap
left a comment
brenard-irap
left a comment
There was a problem hiding this comment.
Thanks, Richard.
I really appreciate the code factorization.
My remarks are mostly aligned with those in SciQLop/PyISTP#1
I'll try to find some additional files for testing.
| if dim_name not in ds.variables: | ||
| var = ds.createVariable(dim_name, 'f8', (dim_name,)) | ||
| var.units = "seconds since 1970-01-01T00:00:00" | ||
| var.VAR_TYPE = "support_data" |
There was a problem hiding this comment.
It may be worth considering whether multiple options for the time format should be supported here.
The ISTP specification tends to recommend TT2000, but I’m not convinced it is necessarily more appropriate than a Unix timestamp.
| if (var is not None) and (var.values.shape[0] == var.axes[0].values.shape[0]): | ||
| time_axis_name = var.axes[0].name | ||
| return _valid_variable_or_none(SpeasyVariable( | ||
| axes=[VariableTimeAxis(values=var.axes[0].values.copy(), |
There was a problem hiding this comment.
Regarding what I mentioned in SciQLop/PyISTP#1 (review), this would be the right place to delegate to the codec the interpretation and conversion of the time variable data into datetime64.
| from speasy.core.codecs import get_codec | ||
|
|
||
| AC_MFI = os.path.join(os.path.dirname(__file__), "resources", "ac_h2s_mfi_cdaweb.nc") | ||
|
|
There was a problem hiding this comment.
I’ll try to provide you with additional test cases.
However, there are far fewer publicly available datasets in netCDF compared to CDF...
- eliminate duplication moving helpers into istp/__init__.py - create istp/cdf.py and istp/netcdf.py
accept timedelta Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- _write_variable: return type was bool but never returned a value, changed to None - _already_in_cdf: already returns _ax.name (str), so a.name was a bug, use a directly - save_variables: replace hasattr(file, 'write') with isinstance(file, io.IOBase) for Pylance narrowing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
copy/paste issue inherited from istp/ refactoring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a19e9e2 to
6f0a4f1
Compare
|
- Log exception at debug level
- rewrite except clause
pycdfpp expects bytes when loading from a buffer, not memoryview.
|
See #209