Skip to content

Fix handling missing/non-string CF axis units #705

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.

Sign up for GitHub

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

Merged
djhoese merged 1 commit into from
Mar 17, 2026
Merged

Fix handling missing/non-string CF axis units #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pyresample/test/test_utils/test_cf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,26 @@ def test_load_cf_latlon(self, file_func, kwargs, exp_lat, exp_lon, future_geomet
_validate_lonlat_cf_area(adef, cf_info, exp_lon, exp_lat)
assert_future_geometry(adef, future_geometries)

def test_load_cf_axis_without_units(self):
cf_file = _prepare_cf_nh10km()
del cf_file['xc'].attrs['units']
del cf_file['yc'].attrs['units']

_, cf_info = load_cf_area(cf_file, variable='ice_conc')

assert cf_info['x']['unit'] is None
assert cf_info['y']['unit'] is None

def test_load_cf_axis_with_non_string_units(self):
cf_file = _prepare_cf_nh10km()
cf_file['xc'].attrs['units'] = 1
cf_file['yc'].attrs['units'] = 1
Comment on lines +288 to +289
Copy link
Copy Markdown
Member

@djhoese djhoese Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure non-strings are valid CF or maybe that is just up to udunits? I'm tempted to say this shouldn't be supported or am I missing something.

Copy link
Copy Markdown
Contributor Author

@Zaczero Zaczero Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code would crash if a non-string was passed. This test makes it that such non-string values are simply ignored instead of crashing the program.

Copy link
Copy Markdown
Member

@djhoese djhoese Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Seems reasonable. Maybe I'd like a warning...? Regardless, this test seems to be failing in CI. Could you try fixing it? Thanks.


_, cf_info = load_cf_area(cf_file, variable='ice_conc')

assert cf_info['x']['unit'] is None
assert cf_info['y']['unit'] is None

def test_load_cf_dataset_input_decodes_cf_coordinates(self, tmp_path):
import xarray as xr

Expand Down
3 changes: 1 addition & 2 deletions pyresample/utils/cf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ def _load_cf_axis_info(nc_handle, coord_varname):
unit = None

# some units that are valid in CF are not valid to pass to proj
if unit.startswith('rad') or \
unit.startswith('deg'):
if not isinstance(unit, str) or unit.startswith(('rad', 'deg')):
unit = None

# return in a dictionnary structure
Expand Down
Loading