ucum-lhc icon indicating copy to clipboard operation
ucum-lhc copied to clipboard

feat: handle eq <-> mol and eq <-> mass conversions

Open cfu288 opened this issue 10 months ago • 2 comments

This PR addresses #18 and adds handling of eq <-> mol and eq <-> mass conversions.

Changes:

  1. Update of convertUnitTo signature to take an optional object parameter from:

    convertUnitTo(fromUnitCode, fromVal, toUnitCode, suggest?: boolean, molecularWeight?: number)
    

    to:

    convertUnitTo(fromUnitCode, fromVal, toUnitCode, options?: {
      suggest?: boolean;
      molecularWeight?: number;
      valence?: number;
    })
    

    Note that this is a breaking change for existing users - will likely need a semvar change. I've left the previous implementation in as convertUnitTo_legacy. However, if we want full backwards compatibility with the function handling both an options object or the previous multiple args, I can add that back in.

    All tests calling convertUnitTo were updated to accept this new signature.

  2. Update convertUnitTo to handle eq <-> mol and and eq <-> mass conversions

    This was implemented by adding a "equivalentExp_" property to the unit class by first updating the source/ucumXmlDocument.js script and regenerating definitions, and then adding special handling if "equivalentExp_" is detected similarly to how "moleExp_" is handled.

    (Note: as we discussed offline, it doesn't seem like "moleExp_" is currently being handled correctly. I don't attempt to fix that with this PR. "equivalentExp_" currently mimics the existing behavior of "moleExp_", it only acts as a boolean flag. I was thinking about fixing this in a separate PR)

    Related tests have been added to test/testUcumLhcUtils.spec.js

cfu288 avatar Apr 16 '24 16:04 cfu288

Note: I've updated the function to currently take a charge instead of a valence:

convertUnitTo(fromUnitCode, fromVal, toUnitCode, options?: {
  suggest?: boolean;
  molecularWeight?: number;
  charge?: number;
})

cfu288 avatar Apr 22 '24 18:04 cfu288

Note I've excluded any build artifacts from this PR. An npm run build will be required before any release.

cfu288 avatar Apr 24 '24 18:04 cfu288