Hello David,

the script you gave me works fine now, and I've been able to modify it to adjust the names of variables in the output netcdf file by using either nc_set_variable or nc_set_dimension, with one exception: there is a grid_mapping variable which is generated with the name rotated_latitude_longitude and carries the datum: I can't find a construct which corresponds to this variable. Is there a way of changing the name-in-file of the grid_mapping variable?

Hi @sadielbartholomew here are some thoughts for refactoring the constants access situation so that we can most effortlessly use it in cf-python. The user-facing API is unchanged. What do you think?

The on-line docs are unchanged. There is a small change to the help(cfdm.rtol) docs, but tolerable, I think.

cf-python would use it as follows:

# in cf/functions.py

class rtol(cfdm.rtol):
    _CONSTANTS = CONSTANTS
    _Constant = Constant


rtol.__doc__ = cfdm.rtol.__doc__.replace('cfdm.', 'cf.')

So cf-python need not know anything about the functionality nor API nor docs from cfdm.

This is just one idea of many, I'm sure.

Initial work towards doctesting has implied, and after further investigation I can confirm, that the copy method for the ABC Container (i.e. cfdm.core.Container.copy) that is documented as being a deep copying operation is in fact only displaying the behaviour of a shallow copy.

For example, note how the setting of a _custom dict component of g is also reflected in f when it is an item within a container, appearing to be a reference rather than a copy of that item, but not reflected in ff when a simple object:

>>> # Setup
>>> import cfdm
>>> f = cfdm.core.abstract.container.Container()
>>> f._custom
{}
>>> f._custom['feature'] = ['f']
>>> f._custom
{'feature': ['f']}

# Apply the copy, expecting it to be deep
>>> g = f.copy()
>>> g._custom['feature'][0] = 'g'
>>> g._custom
{'feature': ['g']}

# ...but note how the change is also reflected in f:
>>> f._custom
{'feature': ['g']}

# ...though changing the top-level value for g does not influence f:
>>> g._custom['feature'] = 'gee whiz'
>>> g._custom
{'feature': 'gee whiz'}
>>> f._custom
{'feature': ['g']}

Environment

>>> cfdm.environment(paths=False)
Platform: Linux-4.15.0-54-generic-x86_64-with-glibc2.10
HDF5 library: 1.10.6
netcdf library: 4.7.4
Python: 3.8.5
netCDF4: 1.5.4
numpy: 1.19.4
cfdm.core: 1.8.8.1
cftime: 1.3.0
netcdf_flattener: 1.2.0
cfdm: 1.8.8.1

Updates to our workflow to run the test suite, aiming to upload a coverage report to Codecov for two jobs, one each for the latest distros for Ubuntu and Mac using Python 3.7 as a representative Python version (I think it would be superfluous & a waste of Actions jobs to upload further coverage reports, since each job should give a very similar if not identical figure).

Testing in this PR to try to get the two coverage reports to show as a pair of 'checks' tied to the PR. Once I get that working, I can merge the PR.

Resolves #31. This sets up minimal logging to delegate all applicable library messages to the logging module, such that they emerge as before by default, but there is new functionality in that they can be filtered out by severity level via a configurable global log severity level addressing the root logger, however the verbose keyword argument in a method will behave as before by overriding the log level.

Implementation

The main code changes here to achieve the above are:

• loggers, which are instantiated on a modular basis, as is recommended practice, with all inheriting from the root logger initiated in the __init__.py (if we add further logging configuration, we should move this to a dedicated module e.g. logging-config.py);
• a new function ~~cfdm.LOG_SEVERITY_LEVEL~~ [now renamed simply cfdm.LOG_LEVEL for clarity to reflect that the levels are a mixture of verbosity- & severity- based] to change the minimal level at which log messages emerge, being WARNING by default:
  • I have written it so it accepts either the named (case insensitive) words for the levels, e.g. WARNING, INFO, or instead an integer from ~~1 through to 5~~ [now -1 to 3 in a revised schema - see later comment] mapped to those, which is easier for users to recall, though less explicit;
• a new decorator _manage_log_level_via_verbosity which provides the interface with decorated methods' verbose keyword argument, such that, as agreed, if verbose is:
  • ~~True: will display all log messages from that method & any method it calls;~~
  • None, as is now the default for all such methods: will use the global log severity level to determine which messages to display so some at lower levels can be filtered out if configured as such;
  • ~~False: will not display any log messages from that method or any method it calls.~~
  • [Edit:] verbose now takes integer levels consistent with those supported by cfdm.LOG_LEVEL for increased granularity of per-function verbosity control, see https://github.com/NCAS-CMS/cfdm/pull/34#issuecomment-626738123 though note we decided on a different schema, as above
• all previous if verbose: print(<message>) statements being changed to log.<level>(<message>) messages, where the conditional on verbose is no longer required as the decorator handles the equivalent logic (& more).

Work still TODO for this PR

If I have missed anything, let me know! But at the least I still want to:

• [x] set the levels of calls appropriately, since I have started by setting them all to info;
• [x] document the configurability of the logging & the overriding verbose keyword argument in a new sub-section of the docs;
• [x] amend the docstrings of methods with verbose kwargs to indicate the interface with the log severity level;
• [x] add some testing of the emerging log levels, & of the new function & decorator.

Post-PR work

1. With this PR, log messages go, as before, to STDOUT as pure messages i.e. no extra metadata such as datetime stamps or module names are included, however such extensions can now be trivially set up in new handlers e.g. file handlers with full detail to dedicated log files for user support purposes etc., if we wish.
2. This PR does not add new logging calls, it simply replaces current print calls, and now the basis is in place we should add meaningful messages across the codebase at applicable levels.
3. Improved display of objects for readability. I have added a few pretty-print calls already to do this in some cases (e.g. see pprint.pformat in netcdfread.py).

There are cases where a.equals(b) evaluates differently to b.equals(a), at least with a and b being fields as I recently noticed during the work towards append mode (#69), namely for cases where the fields in question are the same except for one having a component missing. In other words, our equals method appears to not be commutative/symmetric for certain operands.

See below for details of the particular case which I observed to give different results (confirmed on the master) depending on the order of fields as class upon which the method acts or the parameter.

This raises some questions for me, because I found the difference in output of a * b and b * a confusing given equality in a logical sense should, to me (e.g. it's certainly the case in a mathematical sense), imply commutative behaviour. The relevant parts of the documentation did not seem to provide any information or clues as to whether the equality method should be symmetric or not, but I thought:

Equality is strict by default.

suggests it should, though perhaps (my emphasis):

Any type of object may be tested but, in general, equality is only possible with another object of the same type, or a subclass of one

could be relevant?

My questions are:

1. Is a difference in result for a.equals(b) and b.equals(a) something that should be possible, particularly in cases such as that outlined below, or is it a bug we should fix?
2. In either case, I think we should add a few lines to the documentation to explicitly outline whether these cases are possible and for what inputs and what constructs the equals method is bound to, so there is no ambiguity.
3. If it is a bug and we ensure symmetrical behaviour, could we and should we make it configurable so a field and another that is the same but reduced can be treated as equal if users desire in some context, e.g. with a kwarg called something like accept_subset?

Example case

Note this example with one field a and a field b that is the same but missing a time dimension coordinate:

...
>>> a.dump()
------------------------------------------------------------------
Field: air_potential_temperature (ncvar%air_potential_temperature)
------------------------------------------------------------------
Conventions = 'CF-1.8'
standard_name = 'air_potential_temperature'
units = 'K'

Data(time(36), latitude(5), longitude(8)) = [[[210.7, ..., 286.6]]] K

Cell Method: area: mean

Domain Axis: air_pressure(1)
Domain Axis: latitude(5)
Domain Axis: longitude(8)
Domain Axis: time(36)

Dimension coordinate: time
    standard_name = 'time'
    units = 'days since 1959-01-01'
    Data(time(36)) = [1959-12-16 12:00:00, ..., 1962-11-16 00:00:00]
    Bounds:Data(time(36), 2) = [[1959-12-01 00:00:00, ..., 1962-12-01 00:00:00]]

Dimension coordinate: latitude
    standard_name = 'latitude'
    units = 'degrees_north'
    Data(latitude(5)) = [-75.0, ..., 75.0] degrees_north
    Bounds:Data(latitude(5), 2) = [[-90.0, ..., 90.0]] degrees_north

Dimension coordinate: longitude
    standard_name = 'longitude'
    units = 'degrees_east'
    Data(longitude(8)) = [22.5, ..., 337.5] degrees_east
    Bounds:Data(longitude(8), 2) = [[0.0, ..., 360.0]] degrees_east

Dimension coordinate: air_pressure
    standard_name = 'air_pressure'
    units = 'hPa'
    Data(air_pressure(1)) = [850.0] hPa
>>> b.dump()
--------------------------------------------------------------------
Field: air_potential_temperature (ncvar%air_potential_temperature_1)
--------------------------------------------------------------------
Conventions = 'CF-1.8'
standard_name = 'air_potential_temperature'
units = 'K'

Data(ncdim%time_1(36), latitude(5), longitude(8)) = [[[210.7, ..., 286.6]]] K

Cell Method: area: mean

Domain Axis: air_pressure(1)
Domain Axis: latitude(5)
Domain Axis: longitude(8)
Domain Axis: ncdim%time_1(36)

Dimension coordinate: latitude
    standard_name = 'latitude'
    units = 'degrees_north'
    Data(latitude(5)) = [-75.0, ..., 75.0] degrees_north
    Bounds:Data(latitude(5), 2) = [[-90.0, ..., 90.0]] degrees_north

Dimension coordinate: longitude
    standard_name = 'longitude'
    units = 'degrees_east'
    Data(longitude(8)) = [22.5, ..., 337.5] degrees_east
    Bounds:Data(longitude(8), 2) = [[0.0, ..., 360.0]] degrees_east

Dimension coordinate: air_pressure
    standard_name = 'air_pressure'
    units = 'hPa'
    Data(air_pressure(1)) = [850.0] hPa
>>> a.equals(b)
False
>>> b.equals(a)
True
>>> a.equals(b, verbose=-1)
Constructs: Comparing <DimensionCoordinate: time(36) days since 1959-01-01 >, <DimensionCoordinate: latitude(5) degrees_north>: 
Constructs: Can't match constructs spanning axes ['time']
Constructs: Can't match <DimensionCoordinate: time(36) days since 1959-01-01 >
Constructs: Can't match <DimensionCoordinate: time(36) days since 1959-01-01 >
Constructs: Can't match <DimensionCoordinate: time(36) days since 1959-01-01 >
Field: Different metadata constructs
False

The _manage_log_level_via_verbosity decorator was only applicable and functioning for use on methods, not for functions not bound to classes, but there are a number of cases of the latter in both cfdm and cf where we want to apply it. This PR:

• confirms the above via extending the decorator unit test so there is failure when it is applied to a function; &
• generalises the decorator so it can be used in any case, fixing the new test failures (this was easy as self can be included in the *args already included in the calls, so I just had to remove the explicit reference to self).

As observed in the unit test for append mode, #69, the seventh (index 6) example field can't be written to 'NETCDF3_CLASSIC' format due to having int64 data, whereas the rest of the examples are writable to any of the supported netCDF formats:

>>> import cfdm
>>> f = cfdm.example_field(6)
>>> # Writes fine to all of the other formats, e.g:
>>> cfdm.write(f, 'file-to-write-to.nc', fmt='NETCDF3_64BIT_DATA')
>>> cfdm.write(f, 'file-to-write-to.nc', fmt='NETCDF4')
>>> # ... just not NETCDF3_CLASSIC:
>>> cfdm.write(f, 'file-to-write-to.nc', fmt='NETCDF3_CLASSIC')
Traceback (most recent call last):
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 2728, in _write_netcdf_variable
    self._createVariable(**kwargs)
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 2476, in _createVariable
    g["nc"][ncvar] = g["netcdf"].createVariable(**kwargs)
  File "src/netCDF4/_netCDF4.pyx", line 2771, in netCDF4._netCDF4.Dataset.createVariable
  File "src/netCDF4/_netCDF4.pyx", line 3822, in netCDF4._netCDF4.Variable.__init__
  File "src/netCDF4/_netCDF4.pyx", line 1950, in netCDF4._netCDF4._ensure_nc_success
RuntimeError: NetCDF: Not a valid data type or _FillValue type mismatch

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
...
    nc_encodings = self._write_node_count(
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 1693, in _write_node_count
    self._write_netcdf_variable(ncvar, (geometry_dimension,), count)
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 2734, in _write_netcdf_variable
    raise ValueError(
ValueError: Can't write int64 data from <Count: (2) > to a NETCDF3_CLASSIC file. Consider using a netCDF4 format, or use the 'datatype' parameter, or change the datatype before writing.

So we said we would change it so it can be written to all such formats as with all other examples.

As a workaround for now for testing in #69 I added a skip at the relevant place in 15bd0d12f1347c97bc40d02032ab2b3aea992f7f.

It would be really useful to be able to read CDL files directly into cfdm, rather than having to first convert to binary netCDF files. Can this be added?

I've just started using cfdm ... still finding my way around the many classes. I'm trying to implement a vertical coordinate reference system for an atmosphere_hybrid_height_coordinate .. using the "more complete" example in the tutorial (https://ncas-cms.github.io/cfdm/1.7.1/tutorial.html ) .. which shows all the structures I need. In my code I get an error message (copied below) about an unexpected argument. I can reproduce the error if I take your script from the tutorial (which works as it is) and comment out the line "tas.set_construct(horizontal_crs)". Then, as in the script I want to create, you only have a vertical coordinate reference. The script still executes fine. tas.dump() also works as expected, but cfdm.write( "tas.nc", tas ) produces the following:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.5/dist-packages/cfdm/read_write/write.py", line 357, in write
    verbose=verbose)
  File "/usr/local/lib/python3.5/dist-packages/cfdm/read_write/netcdf/netcdfwrite.py", line 3402, in write
    self._write_field(f)
  File "/usr/local/lib/python3.5/dist-packages/cfdm/read_write/netcdf/netcdfwrite.py", line 2699, in _write_field
    self._create_vertical_datum(ref, owning_coord_key)
  File "/usr/local/lib/python3.5/dist-packages/cfdm/read_write/netcdf/netcdfwrite.py", line 2853, in _create_vertical_datum
    datum=self.implementation.get_datum(ref))
TypeError: initialise_CoordinateReference() got an unexpected keyword argument 'coordinates'

The on-line docs at https://ncas-cms.github.io/cfdm/ are for v1.9.0.4, but the files in the docs directory have been correctly updated to v1.10.0.0 (i.e. the current version).

Any ideas, @sadielbartholomew?

Equivalent to NCAS-CMS/cf-python#492 but note that here there are many more (in total ~50) classes which accept the source: parameter (since these get inherited downstream for cf-python which doesn't need to redefine them), so once we decide what to document for that parameter, consistently, we probably want to start by updating them here in cfdm:

$ pwd
/home/sadie/cfdm/cfdm
$ git grep "source:"
auxiliarycoordinate.py:            source: optional
bounds.py:            source: optional
cellmeasure.py:            source: optional
coordinatereference.py:            source: optional
core/abstract/container.py:            source: optional
core/abstract/parameters.py:            source: optional
core/abstract/parameters.py:        if source:
core/abstract/parametersdomainancillaries.py:            source: optional
core/abstract/parametersdomainancillaries.py:        if source:
core/abstract/properties.py:            source: optional
core/abstract/propertiesdata.py:            source: optional
core/abstract/propertiesdatabounds.py:            source: optional
core/bounds.py:            source: optional
core/cellmeasure.py:            source: optional
auxiliarycoordinate.py:            source: optional
bounds.py:            source: optional
cellmeasure.py:            source: optional
coordinatereference.py:            source: optional
core/abstract/container.py:            source: optional
core/abstract/parameters.py:            source: optional
core/abstract/parameters.py:        if source:
core/abstract/parametersdomainancillaries.py:            source: optional
core/abstract/parametersdomainancillaries.py:        if source:
core/abstract/properties.py:            source: optional
core/abstract/propertiesdata.py:            source: optional
core/abstract/propertiesdatabounds.py:            source: optional
core/bounds.py:            source: optional
core/cellmeasure.py:            source: optional
auxiliarycoordinate.py:            source: optional
bounds.py:            source: optional
cellmeasure.py:            source: optional
coordinatereference.py:            source: optional
core/abstract/container.py:            source: optional
core/abstract/parameters.py:            source: optional
core/abstract/parameters.py:        if source:
core/abstract/parametersdomainancillaries.py:            source: optional
core/abstract/parametersdomainancillaries.py:        if source:
core/abstract/properties.py:            source: optional
core/abstract/propertiesdata.py:            source: optional
core/abstract/propertiesdatabounds.py:            source: optional
core/bounds.py:            source: optional
core/cellmeasure.py:            source: optional
core/cellmethod.py:            source: optional
core/cellmethod.py:        if source:
core/constructs.py:            source: optional
core/coordinatereference.py:            source: optional
core/coordinatereference.py:        if source:
core/data/data.py:            source: *optional*
core/domain.py:            source: optional
core/domainaxis.py:            source:
core/field.py:            source: optional
count.py:            source: optional
data/abstract/compressedarray.py:            source: optional
data/abstract/raggedarray.py:            source: optional
data/data.py:            source: optional
data/gatheredarray.py:            source: optional
data/netcdfarray.py:            source: optional
data/raggedcontiguousarray.py:            source: optional
data/raggedindexedarray.py:            source: optional
data/raggedindexedcontiguousarray.py:            source: optional
data/subarray/abstract/subarray.py:            source: optional
data/subarray/abstract/subsampledsubarray.py:            source: optional
data/subarray/gatheredsubarray.py:            source: optional
data/subsampledarray.py:            source: optional
datum.py:            source: optional
dimensioncoordinate.py:            source: optional
domain.py:            source: optional
(cf-env) :)[14:20:08]cfdm>git grep "source:"
auxiliarycoordinate.py:            source: optional
bounds.py:            source: optional
cellmeasure.py:            source: optional
coordinatereference.py:            source: optional
core/abstract/container.py:            source: optional
core/abstract/parameters.py:            source: optional
core/abstract/parameters.py:        if source:
core/abstract/parametersdomainancillaries.py:            source: optional
core/abstract/parametersdomainancillaries.py:        if source:
core/abstract/properties.py:            source: optional
core/abstract/propertiesdata.py:            source: optional
core/abstract/propertiesdatabounds.py:            source: optional
core/bounds.py:            source: optional
core/cellmeasure.py:            source: optional
core/cellmethod.py:            source: optional
core/cellmethod.py:        if source:
core/constructs.py:            source: optional
core/coordinatereference.py:            source: optional
core/coordinatereference.py:        if source:
core/data/data.py:            source: *optional*
core/domain.py:            source: optional
core/domainaxis.py:            source:
core/field.py:            source: optional
count.py:            source: optional
data/abstract/compressedarray.py:            source: optional
data/abstract/raggedarray.py:            source: optional
data/data.py:            source: optional
data/gatheredarray.py:            source: optional
data/netcdfarray.py:            source: optional
data/raggedcontiguousarray.py:            source: optional
data/raggedindexedarray.py:            source: optional
data/raggedindexedcontiguousarray.py:            source: optional
data/subarray/abstract/subarray.py:            source: optional
data/subarray/abstract/subsampledsubarray.py:            source: optional
data/subarray/gatheredsubarray.py:            source: optional
data/subsampledarray.py:            source: optional
datum.py:            source: optional
dimensioncoordinate.py:            source: optional
domain.py:            source: optional
domainancillary.py:            source: optional
domainaxis.py:            source: optional
field.py:            source: optional
fieldancillary.py:            source: optional
index.py:            source: optional
interiorring.py:            source: optional
interpolationparameter.py:            source: optional
list.py:            source: optional
mixin/files.py:            source: optional
mixin/netcdf.py:            source: optional
mixin/propertiesdatabounds.py:            source: optional
nodecountproperties.py:            source: optional
partnodecountproperties.py:            source: optional
tiepointindex.py:            source: optional
$ git grep "source:" | wc -l
53

Possible bug, but also possibly due to floating point precision subtleties. Identified by David during another PR and its review. See https://github.com/NCAS-CMS/cfdm/pull/222#issuecomment-1295208100 for details.

Identified by David during another PR and its review. See https://github.com/NCAS-CMS/cfdm/pull/222#issuecomment-1295197082 for details. The zeroes should not appear and it isn't clear without investigation why they are aren't being printed as masked values like they should be, given the nature of the underlying (masked) data.

Whilst improving some docstrings in #183 I noticed that there were two cases where duplicate integer values were provided as codes in the _code0 dictionary, namely duplication on 200 and 201 with:

https://github.com/NCAS-CMS/cfdm/blob/a9816e02f1640f7c63e8214122b302d1206b68ee/cfdm/read_write/netcdf/netcdfread.py#L35-L36

and

https://github.com/NCAS-CMS/cfdm/blob/a9816e02f1640f7c63e8214122b302d1206b68ee/cfdm/read_write/netcdf/netcdfread.py#L53-L54

where for all other keys in that dictionary, and indeed in the similar dict _code1, the values are unique numbers. Possibly this wasn't noticed before due to the ordering of the key-value pairs, which are ordered with the values strictly increasing except for the former pair of bounds-related cases which were out-of-place.

@davidhassell is this an issue or is there a reason for the duplication? Thanks.

To implement the changed behaviour agreed to be appropriate for Data.equals and non-numeric array inputs in the mini-thread https://github.com/NCAS-CMS/cf-python/pull/254#discussion_r786631596, both for cfdm itself and as passed downstream to cf-python. Namely:

we should allow any two string-like data types to be equal

noting this is (knowingly) in contrast to the behaviour we have chosen for numeric data, where different (numeric) data types will be considered unequal, even if the same data is the same.

In CF-1.9, lossy compression by subsampled coordinates was introduced, and needs to be implemented in cfdm.

#167 deals with most of this, but not the writing of subsampled coordinates to netCDF files. This needs to be dealt with separately, as it may involve a refactor of netcdfwrite.py.