Skip to content

Fix columnMappings keys to dcid: predicate form (#102)#117

Merged
tillywoodfield merged 2 commits into
mainfrom
fix/columnmappings-dcid-keys-102
Jun 25, 2026
Merged

Fix columnMappings keys to dcid: predicate form (#102)#117
tillywoodfield merged 2 commits into
mainfrom
fix/columnmappings-dcid-keys-102

Conversation

@tillywoodfield

Copy link
Copy Markdown
Contributor

Closes #102.

What

ColumnMappings serialized its fields under short Python names (entity, variable, date, …), but the DCP importer keys observations by dcid:-prefixed predicate names. Any other key is treated as a custom dimension, so every exported config.json failed the import:

ValueError: Missing required column mapping for: 'dcid:variableMeasured'

How

Update ColumnMappings to accept either form on input and always emit the dcid: key on output.

field emitted key
variable dcid:variableMeasured
entity dcid:observationAbout
date dcid:observationDate
value dcid:value
unit dcid:unit
measurementMethod dcid:measurementMethod
observationPeriod dcid:observationPeriod
scalingFactor scalingFactor (unchanged — see below)

scalingFactor

Left as the short scalingFactor key because the importer has no dcid:scalingFactor route yet.

Tests

  • test_column_mappings_emit_dcid_keys — asserts the exact emitted key strings and that scalingFactor stays short (guards against alias typos).
  • test_column_mappings_accepts_dcid_keys_on_input — read-back guard.
  • Existing golden/round-trip tests pass against the updated fixture.

Co-authored with Claude Code

@tillywoodfield tillywoodfield marked this pull request as draft June 25, 2026 10:27
@tillywoodfield tillywoodfield force-pushed the fix/columnmappings-dcid-keys-102 branch from 712220b to 130e1a1 Compare June 25, 2026 10:29
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.35%. Comparing base (48aecf8) to head (130e1a1).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   91.06%   93.35%   +2.29%     
==========================================
  Files          37       38       +1     
  Lines        1801     2048     +247     
==========================================
+ Hits         1640     1912     +272     
+ Misses        161      136      -25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tillywoodfield tillywoodfield marked this pull request as ready for review June 25, 2026 10:32
@tillywoodfield tillywoodfield requested a review from jm-rivera June 25, 2026 10:32
@tillywoodfield tillywoodfield self-assigned this Jun 25, 2026
@tillywoodfield tillywoodfield force-pushed the fix/columnmappings-dcid-keys-102 branch from 130e1a1 to a0e3dc1 Compare June 25, 2026 11:31

@jm-rivera jm-rivera left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @tillywoodfield! it may be worth fixing a small linked issue. Since columnMappings is consumed by the importer, which requires the dcid prefixed keys, we need the serialisation to always work with the alias mapping.

To solve it, please just add by_alias=True to the model_dump call in config_to_dict(). You may have to update a related test that may not be expecting the dcid prefix on the keys.

@tillywoodfield tillywoodfield force-pushed the fix/columnmappings-dcid-keys-102 branch from a0e3dc1 to 9d20ba0 Compare June 25, 2026 12:39
@tillywoodfield tillywoodfield requested a review from jm-rivera June 25, 2026 12:40

@jm-rivera jm-rivera left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @tillywoodfield !

@tillywoodfield tillywoodfield merged commit c61cb92 into main Jun 25, 2026
3 checks passed
@tillywoodfield tillywoodfield deleted the fix/columnmappings-dcid-keys-102 branch June 25, 2026 12:53
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.

Fix columnMappings keys to the importer's dcid: predicate form

3 participants