8000 fix: report EC as a range, never refuse or block (G6-G9) by rafatahmed · Pull Request #225 · rafatahmed/Flahax · GitHub
[go: up one dir, main page]

Skip to content

fix: report EC as a range, never refuse or block (G6-G9) - #225

Merged
rafatahmed merged 1 commit into
mainfrom
fix/ec-range
Nov 6, 2026
Merged

rafatahmed merged 1 commit into
mainfrom
fix/ec-range

Conversation

@rafatahmed
Copy link
Copy Markdown
Owner

Summary

 calculate_ec_from_totals
-  species without coefficient      -> partial (subtotal only)
-  phosphate beyond fit I           -> not_supported
-  assay residual too large         -> not_supported
-  unrecorded counterions           -> partial
+  low  = sum of species with a coefficient
+  high = low + 80 x |z| x m for uncovered species, phosphate beyond the fit,
+         and charge a supplied pH leaves unbalanced
+  low == high -> estimated (one value); else range (ec25_ms_cm None, no midpoint)
+  not_supported only when the state cannot be solved
  • flahax batch --ec X / summarize_batch(requested_ec_ms_cm=X): "compatible with the estimated range; target attainment unverified", or "requested X is Y mS/cm from the estimated range". Never enforced.
  • The model note (McCleskey 2012 accuracy is for natural waters; measure EC after mixing) is always shown.
  • Report, sheet, examples, docs, changelog and 0.7.0 notes updated; no midpoint anywhere.

Evidence

  • Before: Lettuce 2 (Howard Resh), RO water: ec.status = not_supported, reason ionic_strength_outside_phosphate_fit (I = 0.0262).
    After: ec.status = range, ec25_range_ms_cm = (low, high), low < high.
  • New tests/test_ec_range.py (8) and tests/test_ec_report.py (10); 7 existing modules rewritten to the new contract. Scoped coverage of every changed line and branch: no gaps.
  • PHREEQC golden fixture: not affected (EC output only; no conversion or speciation numbers change).

Merge Danger

Door: one-way for API/file consumers.

Removed: status partial, the assay and phosphate-fit refusals, reasons assay_charge_residual_too_large, missing_species_contributions, unrecorded_counterions. Consumers must read status (estimated/range) and ec25_range_ms_cm; ec25_ms_cm is None for a range.

Blast Radius: EC fields in summaries, report, batch sheet, examples, CLI.

Deviations to review:

  • Model note uses ASCII hyphen -0.7 instead of the plan's U+2212, which cannot be encoded on latin-1/cp1252 consoles.
  • The 80 per charge bound comes from the plan ("Adamson"); I could not verify the source.
  • The unrecorded-charge bound under a supplied pH and the --ec flag go slightly beyond the plan's wording.
  • Not in scope: calculated_ec.py still returns missing_product_ec_evidence. Optional smells from review (duplicate ec_text calls, three range formatters) are left as follow-ups.

Closes #224

EC is now `estimated` (one value) or `range` (low, high; no midpoint). Species
without a conductivity coefficient, phosphate beyond its fit ionic strength and
charge left unbalanced by a supplied pH are bounded at 80 per charge instead of
refusing. `not_supported` remains only when the state cannot be solved. A
requested EC (`flahax batch --ec`) is compared with the range, never enforced.

Closes #224
@rafatahmed rafatahmed added the bug Something isn't working label Nov 6, 2026
@rafatahmed rafatahmed self-assigned this Nov 6, 2026
@read-the-docs-community
Copy link
Copy Markdown

@codecov
codecov Bot commented Nov 6, 2026 •
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (976864a) to head (c1b118d).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #225   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines        13111     13130   +19     
  Branches      2570      2573    +3     
=========================================
+ Hits         13111     13130   +19     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@rafatahmed
rafatahmed marked this pull request as ready for review November 6, 2026 10:24
@rafatahmed
rafatahmed merged commit f6bbb75 into main Nov 6, 2026
13 checks passed
@rafatahmed
rafatahmed deleted the fix/ec-range branch November 6, 2026 10:24
rafatahmed added a commit that referenced this pull request Nov 7, 2026
fix: report EC as a range, never refuse or block (G6-G9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EC: report a range, never refuse or block (G6–G9)

1 participant

0