Skip to content

Fix CompositeGenericTransform.contains_branch_separately ignoring first child#32109

Open
AnishPatel526 wants to merge 1 commit into
matplotlib:mainfrom
AnishPatel526:fix/contains-branch-separately-32099
Open

Fix CompositeGenericTransform.contains_branch_separately ignoring first child#32109
AnishPatel526 wants to merge 1 commit into
matplotlib:mainfrom
AnishPatel526:fix/contains-branch-separately-32099

Conversation

@AnishPatel526

Copy link
Copy Markdown

Fix CompositeGenericTransform.contains_branch_separately ignoring the first child (closes #32099)

PR summary

CompositeGenericTransform.contains_branch_separately only inspected the
second (suffix) child, self._b, so any branch located in the first child
self._a — or one that spans the boundary between the two children — was
reported as absent. This made the per-dimension result inconsistent with
contains_branch, which correctly detects such branches.

Reproduction (matplotlib 3.10.9)

import numpy as np
import matplotlib.transforms as mt

class NonAffineForTest(mt.Transform):
    is_affine = False; input_dims = output_dims = 2
    def __init__(self, real, *a, **k):
        self.real_trans = real; super().__init__(*a, **k)
    def transform_non_affine(self, v): return self.real_trans.transform(v)
    def transform_path_non_affine(self, p): return self.real_trans.transform_path(p)

ta1 = mt.Affine2D().rotate(np.pi/2); ta2 = mt.Affine2D().translate(10, 0)
ta3 = mt.Affine2D().scale(1, 2); tn1 = NonAffineForTest(mt.Affine2D().translate(1, 2))
stack  = ta1 + tn1 + ta2 + ta3        # CompositeGenericTransform
subset = tn1 + ta2 + ta3              # a genuine suffix branch

stack.contains_branch(subset)             # -> True
stack.contains_branch_separately(subset)  # -> (False, False)   BUG (inconsistent)

Fix

Query the suffix child first (so a blended child can still report distinct
per-dimension results), and when it does not already contain the branch in both
dimensions, fall back to the whole-transform contains_branch check. A branch
found that way is a complete suffix of the composite and therefore applies to
both dimensions.

x, y = self._b.contains_branch_separately(other_transform)
if not (x and y):
    in_branch = self.contains_branch(other_transform)
    x = x or in_branch
    y = y or in_branch
return (x, y)

A naive alternative — ORing self._a and self._b's separate results — does
not work, because a branch that straddles the split point (e.g. subset
above) is a suffix of neither child individually; only the whole-transform
check catches it.

After the fix the reproduction returns (True, True), matching
contains_branch.

Behaviour preserved

  • Blended per-dimension results are unchanged: the existing
    test_contains_branch case stack_blend.contains_branch_separately(...)
    still returns (False, True).
  • Non-branch transforms still return (False, False) — no over-reporting.

Tests

Extended TestBasicTransform.test_contains_branch in
lib/matplotlib/tests/test_transforms.py with cases for a branch in the first
child, a branch spanning the child boundary, and a non-branch. These fail on
main and pass with this change.

AI Disclosure

AI assistance was used to investigate the codebase and draft this change and
its tests; the logic was verified manually against the released build.

PR checklist

@github-actions

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: CompositeGenericTransform.contains_branch_separately only checks second component, ignoring first

1 participant