Skip to content

Conversation

lipchev
Copy link
Collaborator

@lipchev lipchev commented Jan 3, 2025

Ensure that all base quantity units are BaseUnits:

  • introduced a test for each of the base quantities (Length, Mass etc.) that ensures that all of their units have valid BaseUnits definition
  • fixed some incorrect BaseUnits definitions: Acceleration.StandardGravity and Temperature.MillidegreeCelsius
  • added missing BaseUnits definitions: Length, Temperature and TemperatureChangeRate (removing 3 out of its 4 [Skip]-ed tests)

new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.DegreeFahrenheitPerSecond, "DegreesFahrenheitPerSecond", new BaseUnits(time: DurationUnit.Second, temperature: TemperatureUnit.DegreeFahrenheit), "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.DegreeKelvinPerHour, "DegreesKelvinPerHour", new BaseUnits(time: DurationUnit.Hour, temperature: TemperatureUnit.Kelvin), "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.DegreeKelvinPerMinute, "DegreesKelvinPerMinute", new BaseUnits(time: DurationUnit.Minute, temperature: TemperatureUnit.Kelvin), "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.DegreeKelvinPerSecond, "DegreesKelvinPerSecond", new BaseUnits(time: DurationUnit.Second, temperature: TemperatureUnit.Kelvin), "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.HectodegreeCelsiusPerSecond, "HectodegreesCelsiusPerSecond", BaseUnits.Undefined, "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.KilodegreeCelsiusPerSecond, "KilodegreesCelsiusPerSecond", new BaseUnits(time: DurationUnit.Millisecond, temperature: TemperatureUnit.DegreeCelsius), "TemperatureChangeRate"),
new UnitInfo<TemperatureChangeRateUnit>(TemperatureChangeRateUnit.MicrodegreeCelsiusPerSecond, "MicrodegreesCelsiusPerSecond", BaseUnits.Undefined, "TemperatureChangeRate"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok check this out, I found another limitation of the UnitPrefixBuilder - given that the Temperature now has a the BaseUnit set for MillidegreesCelsius, you'd think that this would get resolved here. However, the reason it doesn't is because there is nothing in Temperature.json that links the MillidegreesCelsius to the prefix Milli (outside the name). The same applies to the TemperatureDelta.g.cs where the MillidegreesCelsius doesn't have any BaseUnits, as it is prefixed, while the base quantity isn't.

I've checked for other conventionally defined prefixes in the base quantities, and found just the Microinch (as well as the Mil, if this is supposed to be Kilo). There isn't anything stopping us from making the Microinch a simple prefix unit:

    {
      "SingularName": "Inch",
      "PluralName": "Inches",
      "BaseUnits": {
        "L": "Inch"
      },
      "FromUnitToBaseFunc": "{x} * 2.54e-2",
      "FromBaseToUnitFunc": "{x} / 2.54e-2",
      "Prefixes": [ "Micro" ],
      "Localization": [
        {
          "Culture": "en-US",
          "Abbreviations": [ "in", "\\\"", "" ],
          "AbbreviationsForPrefixes": { "Micro": "µin" }
        },
        {
          "Culture": "ru-RU",
          "Abbreviations": [ "дюйм" ],
          "AbbreviationsForPrefixes": { "Micro": "микродюйм" }
        },
        {
          "Culture": "zh-CN",
          "Abbreviations": [ "英寸" ],
          "AbbreviationsForPrefixes": { "Micro": "微英寸" }
        }
      ]
    }, 

I tested it locally, hoping to see some other quantity picking up on the newly available prefix, but there weren't any (I think there should be some, but are probably just missing their BaseUnits).

For the Mil yeah, not gonna happen.. And the Temperature, we cannot have it as Prefix since it is an affine quantity. I could of course, hard-code them in the BaseUnitPrefixes 😄

@angularsen angularsen merged commit 7556b7c into angularsen:release/v6 Jan 19, 2025
1 check passed
@lipchev lipchev deleted the base-units-cleanup branch April 11, 2025 20:22
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.

2 participants