Add new net_cdf4 codec#288
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- 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>
| def _try_set_attr(nc_var, key, value): | ||
| try: | ||
| setattr(nc_var, key, value) | ||
| except Exception: |
RichardHitier
force-pushed
the
netcdf_codec
branch
from
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...
3 participants
See #209