Skip to content

fix: let an explicit null in Props.add_to override - #62

Merged
gipert merged 1 commit into
gipert:mainfrom
ggmarshall:fix-add-to-null
Sep 26, 2026
Merged

gipert merged 1 commit into
gipert:mainfrom
ggmarshall:fix-add-to-null

Conversation

@ggmarshall

@ggmarshall ggmarshall commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor
  • I have reviewed all the code in this pull request and deem it of the standard expected for this project.

Since #48, Props.add_to reads props_b with .get(), so an explicit null looks the same as a missing key and the old value is kept. Before #48 (dbetto ≤ 1.3.4), a null in the overlay replaced the value.

LEGEND's dataprod configs rely on this: category overlays set tier_evt: inputs: muon_config: null to switch off the muon evt step for cal/fft/pzc/bkg/hvs. With dbetto ≥ 1.3.5 those runs silently get the muon evt step again.

This PR restores the old rule: only a key absent from props_b keeps the props_a value. It adds test_props_add_to_null_overrides, which fails on main.

Validation. pytest (31 passed), pre-commit run -a and nox -s pylint (10.00/10) all pass. I also resolved TextDB.on() for all 3,643 (validity timestamp × category) lookups in the LEGEND dataprod/config, dataprod/overrides/*, datasets/statuses and hardware/configuration/channelmaps DBs. With this branch dataprod/config is identical to dbetto 1.2.4, and against 1.4.0 the only change is the muon_config key above. Channel maps and statuses are unchanged.

Related: legend-exp/legend-dataflow#173 (callers discarding the new add_to return value).

🤖 Generated with Claude Code

Since gipert#48, add_to read props_b with .get(), so an explicit null was treated
like a missing key and the old value was kept. Overlay files that set a key
to null to switch something off (e.g. `muon_config: null` in the LEGEND
dataprod configs) silently stopped working. Only a key absent from props_b
now keeps the value from props_a, as before gipert#48.

Assisted-by: Generative AI
Copilot AI lite review requested due to automatic review settings September 25, 2026 22:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.46%. Comparing base (5b17622) to head (45ad70a).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   91.72%   92.46%   +0.73%     
==========================================
  Files           6        6              
  Lines         568      597      +29     
==========================================
+ Hits          521      552      +31     
+ Misses         47       45       -2     

☔ 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.

@gipert
gipert merged commit b74ac3b into gipert:main Sep 26, 2026
14 checks passed
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