Audit edge-case warning semantics#25
Merged
Merged
Conversation
bernalde
commented
May 10, 2026
Member
Author
bernalde
left a comment
There was a problem hiding this comment.
Reviewed against issue #23 and the surrounding warning-producing paths. I did not find blocking issues. The PR removes the repository-wide warning filters, keeps expected noisy warnings localized to tests that intentionally exercise those paths, and adds focused coverage for the Rp_finder Tbot == T_sub singular cases. Targeted warning-as-error tests and the practical non-notebook/non-Pyomo test subset pass locally. Merge-ready from my review; GitHub would not allow an APPROVE review from the PR author account.
This was referenced May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pytest.ini.lyopronto.functions.Rp_finderhandle the exactTbot == T_subsingular case explicitly.pytest.warnsor test-localpytest.mark.filterwarningsin the edge-case tests that intentionally exercise infeasible/no-sublimation paths.Warning Filters
calc_unknownRp,calc_knownRp,design_space,opt_Pch, and theRp_finderdivide-by-zeroRuntimeWarningfrompytest.ini.tests/test_calc_unknownRp_coverage.pyfor repeated expectedNo sublimationand setpoint-exhaustion warnings from coverage-only unknown-Rp scenarios.tests/test_coverage_gaps.pyfor expected design-space infeasibility and single-timestep warnings.tests/test_calculators.py::TestEdgeCases::test_very_low_shelf_temperatureto assert the expected known-Rp vapor-pressure warning withpytest.warns.Rp_finder
Rp_findernow treats a zero bottom-to-sublimation temperature gradient as a documented singular resistance case instead of relying on NumPy divide warnings:NaNfor the indeterminate0/0case;Tests Run
pytest tests/test_functions.py tests/test_calc_unknownRp.py tests/test_calc_knownRp.py tests/test_design_space.py tests/test_opt_Pch.py tests/test_calc_unknownRp_coverage.py tests/test_coverage_gaps.py -q -o addopts= -o filterwarnings=error --maxfail=10127 passed in 180.20spython -m pytest tests/test_calculators.py::TestEdgeCases::test_very_low_shelf_temperature tests/test_opt_Pch_coverage.py tests/test_opt_Pch_Tsh.py::TestOptPchTshValidation::test_joint_optimization_faster_than_single tests/test_opt_Pch_Tsh_coverage.py::TestOptPchTshComparison::test_joint_opt_vs_pch_only -q -o addopts= -o filterwarnings=error16 passed in 111.95spython -m pytest tests/ -n auto -q -m "not notebook and not pyomo" --ignore=tests/test_pyomo_models257 passed in 359.27sgit diff --checkpython -m mypy lyopronto/functions.pySuccess: no issues found in 1 source fileNotes
python -m ruff checkon the changed files was run and reports existing legacy style/import/trailing-whitespace issues across these files; those are outside this warning-semantics change.python -m black --checkon the changed files was run and reports that the legacy files would be reformatted; no formatting pass was applied to avoid unrelated churn.Closes #23