Conflict in unit alias #543
Comments
|
Line 95 in 22ee634 This line is the issue. 'C' is already registered as 'coulomb'. But it aliases 'C' to temperature. Can you remove the 'C' alias?, i.e., units.define("@alias degC = deg_C = Celsius = degrees_Celsius")I'd be happy to submit a quick PR for this, if the removal doesn't affect anything else in the package. |
tien-vo
changed the title
|
Do you need any of the other unit definitions in this file at all? |
|
The other aliases don't affect me for now. But I'd expect 'C' to mean Coulomb, or you should mention the breaking alias(es) in the documentation. |
|
By the way, some of the definitions in I haven't tracked all of them, but at least the percentage is introduced in 0.21 (Add "%" preprocessor and percent and ppm units #1661). This should be a separate issue for |
|
Oh hmm.. yeah we can bump pint I guess. What specific unit definitions do you need from this file? |
|
@aulemahal can you take a look please? |
|
@dcherian Sorry I must have not made myself clear. I need line 95 not to alias 'C' to 'degC' because 'C' already means 'coulomb'. |
|
Right, I got that :) I'm curious which of the other definitions are useful. Are you studying lightning? (we assumed no one would use CF units AND Coloumb when we merged that change) |
|
Ah gotcha. I'm doing plasma physics and migrating from If that's a deliberate choice you made, it's ok to close this issue. I'll write my own parser. |
|
I think removing "C" as an alias of "degC" is ok. In fact, As for "pinning" |
maybe I'm mistaken, but doesn't |
|
Oh! I'm sorry, indeed the pyproject says so. Therefore, I would 100% support the changes discussed in this issue! |
|
@aulemahal I'll submit a PR later this week when I have time to spare. Quick question since I didn't pay attention to SPEC0 before: are you saying it is fine to bump to By the way, I found another bug. The custom regexp parser breaks units with number suffix like 'ln10' or 'K_alpha_W_d_20'. Consider the minimal snippet: import functools
import re
import pint
units = pint.UnitRegistry()
pint.set_application_registry(units)
pint.Unit("degree").compatible_units()
# frozenset({<Unit('decade')>, <Unit('grade')>, <Unit('permille')>, <Unit('milliarcsecond')>, <Unit('fine_structure_constant')>, <Unit('zeta')>, <Unit('arcminute')>, <Unit('turn')>, <Unit('radian')>, <Unit('wien_u')>, <Unit('decibel')>, <Unit('ln10')>, <Unit('wien_x')>, <Unit('avogadro_number')>, <Unit('neper')>, <Unit('byte')>, <Unit('K_alpha_W_d_220')>, <Unit('arcsecond')>, <Unit('K_alpha_Cu_d_220')>, <Unit('degree')>, <Unit('steradian')>, <Unit('square_degree')>, <Unit('electron_g_factor')>, <Unit('count')>, <Unit('mil')>, <Unit('octave')>, <Unit('percent')>, <Unit('ppm')>, <Unit('refractive_index_unit')>, <Unit('bit')>, <Unit('tansec')>, <Unit('K_alpha_Mo_d_220')>, <Unit('eulers_number')>, <Unit('pi')>})
pint.Quantity(1.0, "degree").to("ln10")
# <Quantity(0.00757986863, 'ln10')>But with the regex preprocessor: Lines 66 to 71 in 22ee634 '10' is stripped from 'ln10' and it raises an error units = pint.UnitRegistry(
preprocessors=[
functools.partial(
re.compile(
r"(?<=[A-Za-z])(?![A-Za-z])(?<![0-9\-][eE])(?<![0-9\-])(?=[0-9\-])"
).sub,
"**",
),
],
)
pint.set_application_registry(units)
pint.Unit("degree").compatible_units()I'm not an expert on regex but based on this, adding excluding patterns fixes the issue, e.g.: exclude_patterns = "|".join(["ln10", "K_alpha_Cu_d_220", "K_alpha_Mo_d_220", "K_alpha_W_d_220"])
units = pint.UnitRegistry(
preprocessors=[
functools.partial(
re.compile(
rf"^(?!{exclude_patterns})(?<=[A-Za-z])(?![A-Za-z])(?<![0-9\-][eE])(?<![0-9\-])(?=[0-9\-])"
).sub,
"**",
),
],
)
pint.set_application_registry(units)
pint.Unit("degree").compatible_units()
# frozenset({<Unit('bit')>, <Unit('arcminute')>, <Unit('K_alpha_Mo_d_220')>, <Unit('turn')>, <Unit('decibel')>, <Unit('steradian')>, <Unit('pi')>, <Unit('octave')>, <Unit('arcsecond')>, <Unit('milliarcsecond')>, <Unit('decade')>, <Unit('mil')>, <Unit('ln10')>, <Unit('wien_u')>, <Unit('tansec')>, <Unit('eulers_number')>, <Unit('radian')>, <Unit('zeta')>, <Unit('grade')>, <Unit('refractive_index_unit')>, <Unit('K_alpha_W_d_220')>, <Unit('electron_g_factor')>, <Unit('degree')>, <Unit('square_degree')>, <Unit('byte')>, <Unit('permille')>, <Unit('count')>, <Unit('percent')>, <Unit('fine_structure_constant')>, <Unit('avogadro_number')>, <Unit('neper')>, <Unit('ppm')>, <Unit('K_alpha_Cu_d_220')>, <Unit('wien_x')>})
pint.Quantity(1, "degree").to("ln10")
# <Quantity(0.00757986863, 'ln10')>
pint.Quantity(1, "degree").to("K_alpha_W_d_220")
# <Quantity(0.160339401, 'K_alpha_W_d_220')>Not sure these conversions make sense though. 'ln10' isn't what I thought it was. |
|
AFAIU, As for modifying the regex, I see no issue as long as we preserve what was working before. You should know however, that the real convention for units that the CF world has agreed upon is UDUNITS (https://docs.unidata.ucar.edu/udunits/current/, see database at point 6). And |
Take a conversion from electron temperature to thermal velocity as an example:
If we then import
cf_xarray.unitsto enable CF units, per this page, we get the following error:The text was updated successfully, but these errors were encountered: