Skip to content

convert_units does not convert valid_min/valid_max #6411

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

Open
bouweandela opened this issue Apr 18, 2025 · 4 comments · May be fixed by #6416
Open

convert_units does not convert valid_min/valid_max #6411

bouweandela opened this issue Apr 18, 2025 · 4 comments · May be fixed by #6416

Comments

@bouweandela
Copy link
Member

🐛 Bug Report

iris.cube.Cube.convert_units does not convert the valid_min/valid_max attributes. This can lead to the entire cube becoming masked if it is saved and loaded again because the data values are outside the valid range.

How To Reproduce

Steps to reproduce the behaviour:

In [1]: import iris

In [2]: import iris.cube

In [3]: iris.FUTURE.save_split_attrs = True

In [4]: cube = iris.cube.Cube([263], standard_name='air_temperature', units='K')

In [5]: cube.attributes.locals["valid_min"] = 0

In [6]: cube.convert_units("degrees_C")

In [7]: cube.data
Out[7]: array([-10.15])

In [8]: cube.attributes.locals["valid_min"]
Out[8]: 0

In [9]: iris.save(cube, target='tmp.nc')

In [10]: result = iris.load_cube("tmp.nc")

In [11]: result.data
Out[11]: 
masked_array(data=[--],
             mask=[ True],
       fill_value=9.969209968386869e+36,
            dtype=float64)

In [12]: result.attributes.locals["valid_min"]
Out[12]: np.float64(0.0)

Expected behaviour

I would expect convert_units to also convert the valid_min and valid_max attributes.

Environment

  • OS & Version: [e.g., Ubuntu 20.04 LTS]
  • Iris Version: 3.11

Additional context

Click to expand this section...
Please add additional verbose information in this section e.g., code, output, tracebacks, screenshots etc
@bouweandela
Copy link
Member Author

Looking at the code for coordinates, it looks like there is a similar issue there:

iris/lib/iris/coords.py

Lines 715 to 750 in dbeda2f

def convert_units(self, unit):
"""Change the units, converting the values of the metadata."""
# If the coord has units convert the values in points (and bounds if
# present).
# Note: this method includes bounds handling code, but it only runs
# within Coord type instances, as only these allow bounds to be set.
if self.units.is_unknown():
raise iris.exceptions.UnitConversionError(
"Cannot convert from unknown units. "
'The "units" attribute may be set directly.'
)
# Set up a delayed conversion for use if either values or bounds (if
# present) are lazy.
# Make fixed copies of old + new units for a delayed conversion.
old_unit = self.units
new_unit = unit
# Define a delayed conversion operation (i.e. a callback).
def pointwise_convert(values):
return old_unit.convert(values, new_unit)
if self._has_lazy_values():
new_values = _lazy.lazy_elementwise(self._lazy_values(), pointwise_convert)
else:
new_values = self.units.convert(self._values, unit)
self._values = new_values
if self.has_bounds():
if self.has_lazy_bounds():
new_bounds = _lazy.lazy_elementwise(
self.lazy_bounds(), pointwise_convert
)
else:
new_bounds = self.units.convert(self.bounds, unit)
self.bounds = new_bounds
self.units = unit

@bouweandela
Copy link
Member Author

I would be happy to make a pull request to fix this, if you agree that it should be fixed?

@bjlittle
Copy link
Member

@bouweandela This seems reasonable, and may also touch on valid_range too?

If you've got the cycles and the motivation then feel free to unblock your use case 👍

@bouweandela
Copy link
Member Author

Great! I opened a pull request in #6416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants