test: pin alignment contract for variable/expression/constraint bugs#715
test: pin alignment contract for variable/expression/constraint bugs#715MaykThewessen wants to merge 1 commit into
Conversation
|
@MaykThewessen thanks for the PR. Ill add a bunch of similar tests soon. Could you try to cover more datatypes etc with pytest.parameterize, while keeping the test themselves and the parameters readable? |
PyPSA#709 Add xfail(strict=True) regression tests for the coordinate-alignment bug cluster. Each test asserts the expected behaviour (label-aligned operators, coords-driven dimensionality in add_variables) so the contract is locked in test form, and so the fix can flip an xfail to a regression guard in one commit. PyPSA#586's original add_constraints repro no longer fires on master (the path now label-aligns correctly); the surviving arithmetic piece is covered by the PyPSA#707 / PyPSA#708 tests.
ce0db80 to
8ae16d7
Compare
|
Thanks @FBumann — pushed an update (force-with-lease, Coverage matrix:
All 21 parameter cases still Readability choices:
Happy to widen further (e.g. |
|
@MaykThewessen Feel free to share your thoughts on #717 |
Summary
Adds
test/test_alignment_contract.pywith sixxfail(strict=True)regressiontests that pin the intended coordinate-alignment contract reported across the
open alignment-bug cluster (#586, #706, #707, #708, #709).
The bugs all describe the same root issue: linopy's operators and
add_variableshandle coordinate alignment inconsistently — some by label,some positionally, some silently dropping dims. The discussion on each issue
keeps re-litigating what the contract should be. A failing test pins the
answer: every operator aligns by label,
add_variablespreservescoordsorder and shape regardless of bound type.
Issue → test mapping
test_subtract_then_le_matches_direct_lex - a <= 0andx <= abuild the same constrainttest_add_aligns_by_labelx + aaligns by label, not by positiontest_subtract_aligns_by_labelx - aaligns by labeltest_divide_aligns_by_labelx / aaligns by label andx / a == x * (1 / a)test_add_variables_dataarray_bounds_preserve_coords_orderadd_variablesdim order followscoordsforDataArraybounds with missing dimstest_add_variables_pandas_bounds_broadcast_missing_dimadd_variablesbroadcasts pandas bounds to fullcoordsshape#586 is intentionally not included as its own test. Its original repro
(
m.add_constraints(x >= series)with reordered labels) no longer reproduceson
master— theadd_constraintspath now label-aligns correctly. #707'sdescription already notes this: "#586's own snippet … takes the label-aligned
path and no longer reproduces, but the bug is alive in arithmetic." The
surviving arithmetic piece is covered by the #707 / #708 tests above, so
#586 is effectively merged into that coverage.
Why xfail strict=True
Each test asserts the expected behaviour (the bug fixed), not the current
buggy output. As long as the bug reproduces the test xfails; the moment the
fix lands the test starts passing,
strict=Trueturns the XPASS into a CIfailure, and the maintainer is forced to remove the
xfailmark in the samecommit that closes the issue. That is the right state machine: the test
flips from "bug exists" to "regression guard" in one atomic step.
Confirmed by running locally against
master(commit37af4ba):Notes for maintainers
— pinning the contract is the point of this PR, so disagreement here is
exactly the conversation we need. In particular: the divide test pins both
"
/aligns by label" and "x / a == x * (1 / a)". If you'd rather notcommit to the algebraic identity, that line is easy to drop.
fixtures, and use only
pd.Series/xr.DataArray— no solver call, nonetwork I/O. They run in well under a second.
test/test_constraints.py'sexisting
TestConstraintCoordinateAlignmentclass or alongsidetest/test_variables.py, happy to relocate — kept it in a dedicated fileto make the contract-pinning intent obvious.
Related issues: #586, #706, #707, #708, #709 (and #591, which proposes the
strict-alignment convention these tests would lock in).