[FB] [PI-3585] Change default loading unit from "1" to "unknown" (correct branch) #3709
Conversation
| @@ -1651,9 +1651,9 @@ fc_extras | |||
|
|
|||
| ################################################################################ | |||
| def get_attr_units(cf_var, attributes): | |||
| attr_units = getattr(cf_var, CF_ATTR_UNITS, cf_units._UNIT_DIMENSIONLESS) | |||
| attr_units = getattr(cf_var, CF_ATTR_UNITS, cf_units._UNKNOWN_UNIT_STRING) | |||
There was a problem hiding this comment.
I see no need here to reference private bits of cf_units
-- though I admit that is what the existing code is doing !
We should set up our own global for this, with something like UNKNOWN_UNIT_STRING = str(cf_units(None))
There was a problem hiding this comment.
Would it make sense to change all the other references to private bits of cf_units to something similar? i.e. cf_units._NO_UNIT_STRING.
| @@ -0,0 +1 @@ | |||
| * NetCDF files containing variables without units will be given a unit of "unknown" rather than "1" | |||
There was a problem hiding this comment.
This needs fleshing out to make it clearer how it may affect the user.
Something like ...
"When loading data from netcdf-CF files, where a variable has no "units" property, the corresponding Iris object will have ".units='unknown'". Prior to Iris 3.0, these cases defaulted to "units=1".
This applies to cubes, coordinates, and ancillary data."
| @@ -1195,6 +1195,8 @@ fc_extras | |||
| UD_UNITS_LON = ['degrees_east', 'degree_east', 'degree_e', 'degrees_e', | |||
| 'degreee', 'degreese', 'degrees', 'degrees east', | |||
| 'degree east', 'degree e', 'degrees e'] | |||
| UNKNOWN_UNIT_STRING = str(cf_units.as_unit(None)) | |||
There was a problem hiding this comment.
A tiny point, but I do think it would be better to be definite here, and use 'unknown' instead of None.
There was a problem hiding this comment.
Additional:
I think, from the code here, we don't actually need these to match some canonical form, or even be Unit instances, as they are only used as defaults and assigned values.
In that case, you can simply have strings, with your own choice of form, and don't need to wrap them in as_unit at all.
Also, looking again at cf_units, another simple alternative is to use the "unit symbol"s,
which is perhaps clearer and more unambiguous.
With those, you can just have "?" for unknown, and "-" for no-unit,
which at least avoids any uppercase / lowercase uncertainties.
|
Thanks @stephenworsley ! |
… default_units_patch * 'default_units' of https://github.com/SciTools/iris: Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711) Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709) Change default units to "unknown" for all DimensionalMetadata (SciTools#3713) Update docs CubeList.extract method (SciTools#3694) Correct and improve dev-guide section on fixing graphics-tests. (SciTools#3683) New image hashes for mpl 3x2 (SciTools#3682) Switched use of datetime.weekday() to datetime.dayofwk. (SciTools#3687) Remove TestGribMessage (SciTools#3672) Removed iris.tests.integration.test_grib_load and related CML files. (SciTools#3670) Removed grib-specific test to iris-grib. (SciTools#3671) Fixed asv project name to 'scitools-iris'. (SciTools#3660) Remove cube iter (SciTools#3656) Remove test_grib_save.py (SciTools#3669) Remove test_grib2 integration tests (SciTools#3664) Remove uri callback test which is moved to iris-grib (SciTools#3665) 2v4 mergeback picks (SciTools#3668) Remove test_grib_save_rules.py which has been moved to iris-grib (SciTools#3666) Removed ununused skipIf. (SciTools#3632) Remove grib-specific test. (SciTools#3663) Remove obsolete test. (SciTools#3662)
Squashed commit of the following: commit 912f500 Author: stephenworsley <[email protected]> Date: Mon Jun 1 17:52:42 2020 +0100 Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711) commit 3171b95 Author: stephenworsley <[email protected]> Date: Mon Jun 1 16:28:45 2020 +0100 Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709) commit 3fdccac Author: stephenworsley <[email protected]> Date: Mon Jun 1 12:27:23 2020 +0100 Change default units to "unknown" for all DimensionalMetadata (SciTools#3713)
Squashed commit of the following: commit 912f500 Author: stephenworsley <[email protected]> Date: Mon Jun 1 17:52:42 2020 +0100 Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711) commit 3171b95 Author: stephenworsley <[email protected]> Date: Mon Jun 1 16:28:45 2020 +0100 Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709) commit 3fdccac Author: stephenworsley <[email protected]> Date: Mon Jun 1 12:27:23 2020 +0100 Change default units to "unknown" for all DimensionalMetadata (SciTools#3713)
abooton
changed the title
…its_merge * upstream/default_units: fix test (SciTools#3732) Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711) Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709) Change default units to "unknown" for all DimensionalMetadata (SciTools#3713)
* Change default units to "unknown" for all DimensionalMetadata (#3713) * Change default loading unit from "1" to "unknown" (correct branch) (#3709) * Unify saving behaviour of "unknown" and "no_unit" (#3711) * fix test (#3732) Co-authored-by: stephenworsley <[email protected]>
* Change default units to "unknown" for all DimensionalMetadata (#3713) * Change default loading unit from "1" to "unknown" (correct branch) (#3709) * Unify saving behaviour of "unknown" and "no_unit" (#3711) * fix test (#3732) Co-authored-by: stephenworsley <[email protected]>
The same as #3705 except it is targeting a branch specifically for changes to default units. Because of this, the test is changed to no longer require the loading of ancillary variables.