unit_conversion crashes with TypeError when source value is non-numeric string

[csiege]

Bug Report

Summary

convert_units() in linkml_map/functions/unit_conversion.py crashes with TypeError: can't multiply sequence by non-int of type 'float' when a source column value is a non-numeric string (e.g., 'A' used as a missing/coded value in dbGaP data).

Environment

• linkml-map version: 0.4.0
• Python: 3.12
• pint/ucumvert: latest (via dm-bip Docker image)

Reproduction

YAML transform spec (simplified):

value_decimal:
  populated_from: phv00203154
  unit_conversion:
    source_unit: "[lb_av]"
    target_unit: "kg"

Source data: TSV column phv00203154 (ARIC weight in lbs) contains the value 'A' for some rows (likely a missing/not-collected code).

Stack Trace

linkml_map/functions/unit_conversion.py:95 in convert_units
    quantity = magnitude * from_unit_q     # magnitude = 'A' (string)
    return quantity.to(to_unit).magnitude  # pint tries 'A' * 0.4535... -> TypeError

pint/facets/plain/registry.py:1097 in _convert
    value = value * factor   # value='A', factor=0.4535923700000001
    -> TypeError: can't multiply sequence by non-int of type 'float'

Key locals from the actual stack trace:

magnitude = 'A'
from_unit = '1 pound'
to_unit   = '1 kilogram'
factor = 0.4535923700000001

Root Cause

In unit_conversion.py line 93:

quantity = magnitude * from_unit_q

The magnitude parameter arrives as a raw string from the source data (TSV column value). The function does not:

1. Attempt to coerce the string to a numeric type
2. Check if the value is numeric before constructing the pint Quantity
3. Gracefully handle non-numeric values (skip/return None)

Note: The expr: evaluation path in linkml-map/dm-bip does coerce string values to numeric before simpleeval handles them, so expr: "{phv} * 0.453592" would work for numeric rows. But unit_conversion bypasses that coercion path entirely.

Expected Behavior

Non-numeric values should be handled gracefully. Options:

1. Coerce to float first and skip/return None if coercion fails (preferred -- matches expr: path behavior)
2. Raise a more descriptive error that identifies the problematic value and source column
3. Log a warning and return None for non-numeric values

Workaround

Currently, the only workaround is to replace unit_conversion with an explicit expr: that uses the pipeline's upstream string-to-numeric coercion:

# Instead of:
value_decimal:
  populated_from: phv00203154
  unit_conversion:
    source_unit: "[lb_av]"
    target_unit: "kg"

# Use:
value_decimal:
  expr: "{phv00203154} * 0.453592"

This works because the expr: evaluation path coerces strings to numeric before simpleeval processes them. However, this loses the semantic unit metadata and the precise pint conversion factor.

Impact

This blocks all YAML transform files that use unit_conversion where the source column contains any non-numeric coded values (common in dbGaP data where 'A', 'M', etc. represent missing/not-applicable).

Discovered during ARIC cohort pipeline run in dm-bip against bdy_wgt.yaml (body weight in lbs to kg conversion).

[github-actions]

Summary

Bug: convert_units() in src/linkml_map/functions/unit_conversion.py crashes with TypeError when the source value is a non-numeric string (e.g., coded missing-value sentinels like 'A', 'M' common in dbGaP datasets).

Root Cause (code analysis)

At line 93:

quantity = magnitude * from_unit_q

The function signature declares magnitude: float, but it receives raw string values from TSV/CSV source columns without any coercion. When pint internally tries 'A' * 0.4535..., Python raises TypeError: can't multiply sequence by non-int of type 'float'.

The expr: path avoids this because it coerces values to numeric before passing them to simpleeval. The unit_conversion path has no equivalent guard.

Recommended Fix

Add a float-coercion guard at the top of convert_units():

try:
    magnitude = float(magnitude)
except (TypeError, ValueError):
    return None

This matches the behavior of the expr: path and is consistent with the --continue-on-error flag added in the most recent commit (b840a70). A doctest covering the non-numeric case should also be added.

Related Issues

• Allow specify source_unit for unit_conversion in transform spec #69 (closed) — Added source_unit field to the transform spec; this is the feature path that triggers the bug.
• Expand SQLCompiler to cover more of the TransformationSpecification #150 (open) — Proposes compiling unit_conversion to SQL arithmetic; any fix here should be consistent with that planned SQL path (which would naturally handle NULLs via SQL semantics).
• Incremental refactoring of complex functions via extract-on-touch markers #104 (closed) — Established the extract-on-touch refactoring pattern for map_object; if the unit_conversion branch in map_object is touched to fix this, consider extracting it to a private method per that pattern.

Context

Discovered in the dm-bip pipeline (ARIC cohort bdy_wgt.yaml), converting body weight from lbs ([lb_av]) to kg. dbGaP study variables routinely use single-letter codes ('A' = not collected, 'M' = missing) alongside numeric values in the same column. Any unit_conversion spec applied to such columns will hit this crash.

Priority

Medium-high: blocks all unit_conversion usage on real-world dbGaP data. The workaround (expr: with a hardcoded factor) loses semantic unit metadata and pint's precise conversion factors.

@dragon-ai-agent please fix the convert_units function in src/linkml_map/functions/unit_conversion.py to gracefully handle non-numeric magnitude values (return None when float(magnitude) raises TypeError or ValueError), add a doctest for the non-numeric case, and add a pytest test in the appropriate test file.

[amc-corey-cox]

Thanks for the detailed report, @csiege — this is a real issue and good to have documented.

After looking at this more closely, the scope here is narrower than the issue suggests:

What we'll fix in this issue

1. Numeric string coercion: Values like "123" arriving as strings from TSV/CSV columns should be coerced to float before the pint multiplication. That's a legitimate bug at the string→numeric boundary for tabular input.

2. Explicit none_on_string option on unit_conversion: Rather than silently returning None for non-numeric values, we'll add an opt-in flag (something like none_on_string: true) that lets the curator explicitly declare "I know there are non-numeric values here, drop them." This keeps the behavior visible and intentional in the spec.

What we won't do

Silently returning None for unexpected strings (as the issue's "expected behavior" section suggests) is the wrong default — it masks bad data. In biomedical pipelines especially, coded values like 'A' and 'M' mean something, and the curator should be explicit about how they're handled.

What's already in place

The --continue-on-error flag (now on main) already wraps slot-level errors with full context (class, slot, source row) and reports them at the end of a run. So today:

• Default mode: meaningful error identifying the problematic value, slot, and row
• --continue-on-error: skip the bad row, collect errors, report a summary

The bigger picture

For the use case where coded values need to be mapped to their proper meaning (e.g., 'A' → "not collected"), see:

• Extend value_mappings to support expr values #165 — extends value_mappings to support expr values
• Support derived slot bindings and hide for intermediate computations #164 — hide + slot() for cascading multi-stage derivations

These will enable a pattern where you map coded strings explicitly via value_mappings, do unit conversion on numeric values (possibly in a hidden slot), and combine the results.

[amc-corey-cox]

Correction to my example above — is_numeric() and convert_units() don't exist as expression functions (unit conversion is a declarative tag, not an inline function). A realistic workaround today looks more like:

value_decimal:
  expr: "case({phv00203154} == 'A', None, {phv00203154} == 'M', None, float({phv00203154}) * 0.453592)"

Which illustrates exactly why this is awkward — you lose the semantic unit metadata and pint's precise conversion factor, and you have to enumerate every coded value. The fixes scoped to this issue (numeric string coercion + none_on_string opt-in) plus the features in #165/#164 will make this much cleaner.