Skip to content

fix(units): stop aliasing C to degree Celsius - #672

Open
Cherith1222 wants to merge 2 commits into
xarray-contrib:mainfrom
Cherith1222:fix/coulomb-symbol
Open

Cherith1222 wants to merge 2 commits into
xarray-contrib:mainfrom
Cherith1222:fix/coulomb-symbol

Conversation

@Cherith1222

Copy link
Copy Markdown

Why

Importing the CF unit registry redefines the symbol C as degree Celsius. UDUNITS uses C for coulomb, and that redefinition makes a thermal velocity built from electron-volts fail to convert to km/s.

What changed

  • The degree Celsius alias no longer includes C.
  • degC, Celsius, deg_C, and degrees_Celsius still mean degree Celsius.
  • A test checks that C stays coulomb and that the thermal velocity converts.

Scope

This covers the coulomb symbol in the example from #543. It leaves the regex preprocessor and the pint minimum alone.

Tradeoffs

Keeping C as Celsius would preserve the old alias and keep breaking coulomb. Removing it matches UDUNITS. A bare unit string C now means coulomb. The other Celsius names are unchanged.

Blast Radius

Code that quantified a value with the unit string C and meant Celsius will now get coulomb. degC and Celsius are unchanged. The in-repo what's new file points at GitHub releases for changes after February 2023, so this diff does not add a section there.

Verification

  • Before the alias change, ureg("C") was degree Celsius and np.sqrt(100 * ureg.eV / ureg.m_e).to("km/s") raised DimensionalityError.
  • After the change, C stays coulomb, the same conversion is 4193.828809546234 km/s, and degC is still degree Celsius.
  • python -m pytest cf_xarray/tests/test_units.py is 17 passed.

Drafted with Cursor.

Importing the CF unit registry currently makes C mean degree Celsius, so a thermal velocity built from electron-volts no longer converts to km/s.
UDUNITS uses C for coulomb. degC, Celsius, deg_C, and degrees_Celsius still name degree Celsius.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant