Skip to content

Conversation

@dshkol
Copy link
Collaborator

@dshkol dshkol commented Jan 22, 2026

Summary

This PR fixes issues with normalization and factor conversion.

Changes

M1: Percent UOM label inconsistency

  • Changed uom_string == percentage_string to grepl(percentage_string, uom_string)
  • The percentage_string is a regex pattern (^Percent), so exact equality was never matching
  • Now both the value scaling and label update use the same matching logic

M10: Hardcoded column indices in legacy support

  • Replaced meta2[,1] and meta2[,2] with named column access
  • Use meta2[[dimension_name_col]] and meta2[..., dimension_id_col]
  • More robust for locale variations in column names

Deferred

M2: Handle missing attrs in normalize_cansim_values

  • This fix has been deferred to a separate PR
  • The naive fix of extracting cansimTableNumber from data columns caused unintended metadata folding in vector flows
  • Needs more investigation to find a solution that doesn't introduce side effects

Test plan

  • devtools::check() passes with 0 errors, 0 warnings
  • All existing tests pass including data_consistency tests
  • Manual testing of percent normalization

Related Issues

Addresses #147 (Normalization and factor conversion bugs) - partially

🤖 Generated with Claude Code

mountainMath and others added 3 commits November 19, 2025 20:22
- M1: Fix percent UOM label inconsistency - use grepl() instead of
      exact equality to match percentage patterns consistently
- M10: Replace hardcoded column indices with named column access
       in legacy column file support

Note: M2 (handle missing attrs in normalize_cansim_values) deferred
to a separate PR as it requires more investigation - the naive fix
caused unintended metadata folding in vector flows.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mountainMath mountainMath changed the base branch from master to v0.4.5 January 22, 2026 02:07
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.

3 participants