Skip to content

GraphicsLayout: Fix removeItem() regression#3226

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
dnadlinger:fix-layout-stretch-regression
Mar 24, 2025
Merged

GraphicsLayout: Fix removeItem() regression#3226
j9ac9k merged 1 commit intopyqtgraph:masterfrom
dnadlinger:fix-layout-stretch-regression

Conversation

@dnadlinger
Copy link
Copy Markdown
Contributor

This fixes a regression introduced in
54efcbd, where the stretch factors for the row/column previously containing a removed item were zeroed. In general, this is of course incorrect, as there will be other items sharing the same row/column that should not be disturbed. The intention behind that part of the change is not clear to me; this commit simply reverts it.

Fixes #3195.

This fixes a regression introduced in
54efcbd, where the stretch factors for
the row/column previously containing a removed item were zeroed. In
general, this is of course incorrect, as there will be other items
sharing the same row/column that should not be disturbed. The intention
behind that part of the change is not clear to me; this commit simply
reverts it.
@dnadlinger
Copy link
Copy Markdown
Contributor Author

dnadlinger commented Jan 23, 2025

@psyuktha: Your #3167 introduced this regression, but I couldn't figure out what your intention was there – to me, it seems obviously wrong to affect the entire row/column where the removed item was (and in any case, the indices might have shifted). Could you check what is going on here, please?

@dnadlinger
Copy link
Copy Markdown
Contributor Author

Test failures are unrelated to this PR.

@dnadlinger
Copy link
Copy Markdown
Contributor Author

@j9ac9k: Apologies to ping you, but since you merged the PR introducing this regression, I was hoping you could have a quick look. The regression affects all pyqtgraph applications using clear()/removeItem(), so would be important to have fixed before the next release.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Feb 22, 2025

Hi @dnadlinger

Sorry it's taken me so long to get to this. When it comes to undoing PRs (to address regressions), I try and test more carefully to ensure the original issue is addressed as well as the bug from the initial fix.

Can you submit a reproducible example that I can use to work from? Thanks.

@dnadlinger
Copy link
Copy Markdown
Contributor Author

dnadlinger commented Feb 23, 2025

No worries, @j9ac9k, I know the trouble that maintaining open-source libraries brings all too well…

A reproduction case can be found in #3195 (@pijyoi seems to have independently run into the same issue as I did in my project).

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 24, 2025

Closing/Opening to trigger a new CI run.

@j9ac9k j9ac9k closed this Mar 24, 2025
@j9ac9k j9ac9k reopened this Mar 24, 2025
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 24, 2025

Ugh, who merged that change 🙃 The other PR doesn't actually recover the space either... Thanks for the fix @dnadlinger

@j9ac9k j9ac9k merged commit 6c544e1 into pyqtgraph:master Mar 24, 2025
35 checks passed
@dnadlinger dnadlinger deleted the fix-layout-stretch-regression branch March 24, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GraphicsLayoutWidget does not expand its item to use all space

2 participants