Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initiation of a Domain Axis: clarify source API compat. & fix size #311

Open
sadielbartholomew opened this issue Aug 28, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@sadielbartholomew
Copy link
Member

sadielbartholomew commented Aug 28, 2024

Using cfdm.DomainAxis (and filtering downstream to cf-python, so cf.DomainAxis equivalently) with the source keyword to create a Domain Axis construct works fine when the source it itself a Domain Axis, but when it is a coordinate, the produced construct has not size set and is given a generic key name, when these should be inferred from the coordinate data (unless I am missing something and this can't be done), especially from the statement in the documentation for that parameter which says (my bold to highlight):

Convert source, which can be any type of object, to a DomainAxis instance.

All other parameters, apart from copy, are ignored and their values are instead inferred from source by assuming that it has the DomainAxis API. Any parameters that can not be retrieved from source in this way are assumed to have their default value.

To illustrate:

>>> # Use an example field to illustrate with existing constructs - see what we have first
>>> f = cfdm.example_field(1)
>>> f.domain_axes(todict=True)
{'domainaxis0': <DomainAxis: size(1)>, 'domainaxis1': <DomainAxis: size(10)>, 'domainaxis2': <DomainAxis: size(9)>, 'domainaxis3': <DomainAxis: size(1)>}
>>> f.dimension_coordinates(todict=True)
{'dimensioncoordinate0': <DimensionCoordinate: atmosphere_hybrid_height_coordinate(1) m>, 'dimensioncoordinate1': <DimensionCoordinate: grid_latitude(10) degrees>, 'dimensioncoordinate2': <DimensionCoordinate: grid_longitude(9) degrees>, 'dimensioncoordinate3': <DimensionCoordinate: time(1) days since 2018-12-01 >}
>>>
>>> # Now try creating a new Domain Axes with some existing constructs
>>> # This works fine
>>> da1 = f.construct("domainaxis1")
>>> da1
<DomainAxis: size(10)>
>>> new_domain_axes_0 = cfdm.DomainAxis(source=da1)
>>> new_domain_axes_0
<DomainAxis: size(10)>
>> # But this does not
>>> dc1 = f.dimension_coordinate("dimensioncoordinate1")
>>> dc1
<DimensionCoordinate: grid_latitude(10) degrees>
>>> new_domain_axes_1 = cfdm.DomainAxis(source=dc1)
>>> new_domain_axes_1
<DomainAxis: size()>
>>> # Note the above has no size set, but we expect it have size of dc1 namely size 10.
>>> # Note also the key names are generic for the dim coord case, when get set on the field:
>>> f.set_construct(new_domain_axes_0)
'domainaxis4'
>>> f.set_construct(new_domain_axes_1)
'domainaxis5'
>>> f.domain_axes(todict=True)
{'domainaxis0': <DomainAxis: size(1)>, 'domainaxis1': <DomainAxis: size(10)>, 'domainaxis2': <DomainAxis: size(9)>, 'domainaxis3': <DomainAxis: size(1)>, 'domainaxis4': <DomainAxis: size(10)>, 'domainaxis5': <DomainAxis: size()>}
>>> f.dump()
---------------------------------
Field: air_temperature (ncvar%ta)
---------------------------------
Conventions = 'CF-1.11'
project = 'research'
standard_name = 'air_temperature'
units = 'K'

Data(atmosphere_hybrid_height_coordinate(1), grid_latitude(10), grid_longitude(9)) = [[[262.8, ..., 269.7]]] K

Cell Method: grid_latitude(10): grid_longitude(9): mean where land (interval: 0.1 degrees)
Cell Method: time(1): maximum

Field Ancillary: air_temperature standard_error
    standard_name = 'air_temperature standard_error'
    units = 'K'
    Data(grid_latitude(10), grid_longitude(9)) = [[0.76, ..., 0.32]] K

Domain Axis: atmosphere_hybrid_height_coordinate(1)
Domain Axis: grid_latitude(10)
Domain Axis: grid_longitude(9)
Domain Axis: key%domainaxis5()
Domain Axis: ncdim%y(10)
Domain Axis: time(1)
...

As a side note, the size parameter is accepting arguments that aren't integers, which should also be fixed:

>>> # Further problem - can assign invalid sizes
>>> cfdm.DomainAxis(size="bad size, I'm a string!")
<DomainAxis: size(bad size, I'm a string!)>
>>> cfdm.DomainAxis("bad size, I'm a string!")
<DomainAxis: size(bad size, I'm a string!)>

Environment

Platform: Linux-6.5.13-7-MANJARO-x86_64-with-glibc2.40 
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/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/cfdm/core/__init__.py
scipy: 1.14.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/scipy/__init__.py
cftime: 1.6.2 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/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/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/cfdm/__init__.py
@sadielbartholomew sadielbartholomew added the bug Something isn't working label Aug 28, 2024
@sadielbartholomew sadielbartholomew changed the title Initiation of a Domain Axis from source not inferring size and name Initiation of a Domain Axis from source not inferring size & name Aug 28, 2024
@davidhassell
Copy link
Contributor

Hi Sadie, I see where you're coming from, but DimensionCoordinate and DomainAxis do not have a sufficiently similar API in this case, so I think that we shouldn't be surprised that former can't correctly instantiate the latter - in particular there is no DimensionCoordinate.set_size method.

I appreciate that "same API" is slightly vague term. Perhaps we can qualify by saying that "same API for setting the initialisation parameters", or similar

It so happens that dimension coordinates are always 1-d, so it's tempting to think that it's array size could define an axis axis size, but that is clearly not the case for constructs in general - what if the construct had 4-d data?. Is new_domain_axes_1 = cfdm.DomainAxis(dc1.size) OK for you in this case?

As a side note, the size parameter is accepting arguments that aren't integers, which should also be fixed:

I agree!

@sadielbartholomew
Copy link
Member Author

sadielbartholomew commented Aug 29, 2024

Ah, OK, thanks for clarifying David!

I appreciate that "same API" is slightly vague term. Perhaps we can qualify by saying that "same API for setting the initialisation parameters", or similar

Yes, the vagueness is probably the origin of my issue. My interpretation was, a coordinate of any form, as long of course as the coordinates were 1D, should be trivial enough to 'convert' to a Domain Axis with size and name detected. But I wasn't appreciating the subtleties. It would indeed be good to update the phrasing to aim to prevent further confusion from users.

Is new_domain_axes_1 = cfdm.DomainAxis(dc1.size) OK for you in this case?

Yes, though when we catch up today I suspect you will help me realise I don't need to create a domain axis in the first place - in brief, this is for VISION WRF model data work where I have created vertical coordinates emerging as a auxiliary coordinate and ultimately I want the subspace on the z coorindate to use that and not the parametric z coordinate (which isn't working, it is unitless so incomparable anyhow at least I think). My plan was to change the data axes to replace the current parametric z domain axes with the non-parametric calculated one - but hopefully there is a more direct way!

I agree!

I'll change the title here to reflect the much more trivial bug here :)

@sadielbartholomew sadielbartholomew changed the title Initiation of a Domain Axis from source not inferring size & name Initiation of a Domain Axis: clarifying source API compat. & fixing size Aug 29, 2024
@sadielbartholomew sadielbartholomew changed the title Initiation of a Domain Axis: clarifying source API compat. & fixing size Initiation of a Domain Axis: clarify source API compatibility & fix size Aug 29, 2024
@sadielbartholomew sadielbartholomew changed the title Initiation of a Domain Axis: clarify source API compatibility & fix size Initiation of a Domain Axis: clarify source API compat. & fix size Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants