search

NCAS-CMS / cfdm

A Python reference implementation of the CF data model
http://ncas-cms.github.io/cfdm
MIT License
28 stars 11 forks source link

Unexpected attribute value after `write` setting older `Conventions` #294

Open sadielbartholomew opened 5 months ago

sadielbartholomew commented 5 months ago

(I think this is a bug, else the documentation is not clear on the correct behaviour.) The documentation of the Conventions argument to cfdm.write promises that:

By default the current conventions are always included, but if an older CF conventions is defined then this is used instead.

however when I write out a file specifying an older version of the Conventions, e.g. CF-1.6 using a CF-1.11 field and CF-1.11 version of cfdm as per the minimal example below (I tried with resetting that global attribute, also, with nothing changing frm that), it is read back in again with CF-1.11, which to me contradicts the second statement from the above promise - I would expect to see the Conventions property now being set to the older of the two, CF-1.6.

@davidhassell is the above behaviour incorrect, or am misunderstanding the documentation? In the former case we have a bug and in the latter an issue where the documentation is ambiguous or not clear, IMO.

(I haven't yet investigated why this might happen, mostly because I want to check the behaviour is indeed buggy in this way, but I wonder if the numbering of consecutive CF Conventions might be confusing the logic and need accounting for, since 1.11 comes after 1.6 in terms of versions but 1.6>1.11 numerically...)

Minimal example

>>> cfdm.CF()
'1.11'
>>> f = cfdm.example_field(0)
>>> f.properties()["Conventions"]
'CF-1.11'
>>> cfdm.write(f, "delme.nc", Conventions="CF-1.6")
>>> g = cfdm.read("delme.nc")
>>> g[0].properties()["Conventions"]
'CF-1.11'

Environment

This is using the main branch of cfdm via pip editable install) with:

>>> cfdm.environment()
Platform: Linux-6.5.13-7-MANJARO-x86_64-with-glibc2.39 
HDF5 library: 1.14.2 
netcdf library: 4.9.2 
Python: 3.12.0 /home/slb93/miniconda3/envs/cf-env-312/bin/python
netCDF4: 1.6.5 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/netCDF4/__init__.py
numpy: 1.26.4 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/numpy/__init__.py
cfdm.core: 1.11.1.0 /home/slb93/git-repos/cfdm/cfdm/core/__init__.py
scipy: 1.11.3 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/scipy/__init__.py
cftime: 1.6.3 /home/slb93/git-repos/cftime/src/cftime/__init__.py
netcdf_flattener: 1.2.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/netcdf_flattener/__init__.py
cfdm: 1.11.1.0 /home/slb93/git-repos/cfdm/cfdm/__init__.py
davidhassell commented 5 months ago

Hi Sadie - interesting. I'm not sure if the documentation is wrong (so not a code bug), or vice versa. In the latter case we'd have to check that the field being written out did not contain any features that were introduced post the specified version, which would be possible ...

sadielbartholomew commented 5 months ago

Hi David, but then what do we mean by this:

By default the current conventions are always included, but if an older CF conventions is defined then this is used instead.

? I don't understand what 'if an older CF conventions is defined then this is used instead' is implying, if not what I mention I expect from it in my comment above.