fix(v1): fill join-created positions per side; add fill_value= to add/sub/mul/div - #887
Merged
Merged
Conversation
…/sub/mul/div A reindexing join filled both operands from one value, so div's divisor identity (1) also filled the expression's own const — a missing numerator came out as 1/divisor. Fill each side separately: the expression contributes the zero expression, the constant operand fill_value=, defaulting to 'does not apply here'. Only the two division cells change; legacy is untouched. Pins the full outer-join value table as a parametrised test.
The dataset dim order may deviate from the coord dim order, which made coord_sizes disagree with coord_dims and coord_names. Key coord_sizes off coord_dims so all three stay consistent.
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.
Closes # (if applicable).
Note
The following content was generated by AI.
Changes proposed in this Pull Request
Under v1, a reindexing
join=filled both operands from a single value.That value is the constant operand's identity (
0for*,1for/), so italso landed on the expression's own
const: dividing an expression that wasmissing at a label produced
1 / divisorthere. The additive family wasunaffected only because
0is both its identity and its "contributes nothing"value.
Each side is now filled separately:
(no terms,
const0), whatever the operator;fill_value=, new on.add/.sub/.mul/.div(and theirVariablecounterparts). It defaults to "thisoperand does not apply here" —
0for+,-and*, and the same zero rowfor
/.The join keeps deciding the coordinates:
outerstill returns the union, andevery created label stays in the result. What changed is only what those rows
are worth.
fill_value=requires an explicitjoin=(without one there are no createdpositions) and applies to constant operands only — an expression missing at a
label always contributes the zero expression. Both cases raise rather than
being ignored.
expr.div(cost, join="outer", fill_value=1)restores theunscaled term;
expr.add(price, join="outer", fill_value=10)treats anunpriced label as 10.
Absence an operand carries in — from
mask=,.where(),.shift(),.reindex()— is untouched and still propagates under §6. Only the positionsthe join creates are filled.
Behaviour change: the two division cells only. A missing divisor zeroes its
row instead of leaving the term unscaled, and a missing numerator no longer
comes out as
1 / divisor. Addition, subtraction and multiplication areunchanged in both directions, and legacy semantics are untouched throughout.
Outer-join value table (v1), pinned as a parametrised test
Expression
1 * x + 2, constant2, one label missing on one side,join="outer"; values shown at the created label.expr + exprexpr ± constexpr ± constconst= ±2expr * constcoeff0,const0expr * constconst0expr / constcoeff0,const0 — wascoeff1,const2expr / constconst0 — wasconst0.5Verified by reverting
expressions.pyalone: exactly the two division cellsfail, the other ten pass.
Also:
check_join_fill_valueinsemantics.pynext to the other conventionchecks; internally the previously shared fill parameter is split into
nan_fill(legacy's silent NaN fill) andjoin_fill(what the join creates).§7 and §10 of the convention, the migration guide and the release notes are
updated.
Full suite green under both semantics (6347 passed),
ruffclean.Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.