Skip to content

Fix: Parameter tree ignores user-set 'expanded' state#1175

Merged
j9ac9k merged 6 commits intopyqtgraph:developfrom
2xB:fix-1130
May 30, 2020
Merged

Fix: Parameter tree ignores user-set 'expanded' state#1175
j9ac9k merged 6 commits intopyqtgraph:developfrom
2xB:fix-1130

Conversation

@2xB
Copy link
Copy Markdown
Contributor

@2xB 2xB commented Apr 14, 2020

When setting the 'expanded' state of parameters, this change is not applied
in the graphically visible tree. This commit changes that behaviour by
adding a clause in ParameterItem.optsChanged to react to that.

Also, an old and trivial out-commented debugging code line is removed.

Fixes #1130

When setting the 'expanded' state of parameters, this change is not applied
in the graphically visible tree. This commit changes that behaviour by
adding a clause in `ParameterItem.optsChanged` to react to that.

Fixes pyqtgraph#1130
@jaxankey
Copy link
Copy Markdown

While we're at it, is there a similar action to take on the other options, e.g., "enabled" (I've wanted this in the past), "readonly" (maybe useful), "renamable", "strictNaming", and (maybe?!) "type"?

Also, I think the same should be applied to GroupParameterItem, perhaps by calling the ParameterItem.optsChanged from within GroupParameterItem.optsChanged, so as not to duplicate code.

@jaxankey
Copy link
Copy Markdown

(Also, this is great, thanks!)

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Apr 14, 2020

@jaxankey You are right, there is something fundamentally wrong with the update mechanism. I was not aware that there are that many properties that are not handled on update. I will have a look at this tomorrow or the day after, that definitely needs to be fixed.

@jaxankey
Copy link
Copy Markdown

Yeah, there are more than I listed, too, e.g., 'tip'.

Note this seems to fix setOpts and restoreState, but does not fix the inverse problem of someone expanding/collapsing a parameter in the GUI, which currently does not update opts.

@jaxankey
Copy link
Copy Markdown

(And I can't find a signal that triggers when an expand happens. But I give up for today.)

@2xB 2xB marked this pull request as draft April 14, 2020 21:08
@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Apr 14, 2020

@jaxankey The inverse problem will actually be more difficult to solve: If states are synced with the fundamental Parameter, and as we just implemented that change in the Parameter signals all associated ParameterItems, effectively the collapsed/expanded state of all ParameterItems is always synchronized. But users should be able to collapse something in one view and leave it expanded in another view.

2xB added 2 commits April 18, 2020 20:34
As seen in pyqtgraph#1130, there is interest in synchronizing the "expanded" state
of `Parameter`s in `ParameterTree`s. As a default, this would lead to
users being forced to always have multiple `ParameterTree`s to be
expanded in the exact same way. Since that might not be desirable, this
commit adds an option to customize whether synchronization
of the "expanded" state should happen.
…rTrees

Currently, `Parameter` options `renamable` and `removable` are only considered
when building a new `ParameterTree`. This commit makes changes in those
options reflected in the corresponding `ParameterItem`s.
@2xB 2xB marked this pull request as ready for review April 18, 2020 19:13
@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Apr 18, 2020

@jaxankey Made three changes, namely:

  • As a default, synchronizing the "expanded" state would lead to users being forced to always have multiple ParameterTrees to be expanded in the exact same way. To work around that, an option syncExpanded is added to customize whether synchronization of the "expanded" state should happen. This reverts the initial idea of this PR to always pass changes in expanded to ParameterItems, making the synchronization bidirectional once activated.
  • Reflect changes in Parameter options renamable and removable in UI.
  • Reflect changes in Parameter option tip in UI.

Additionally, since changing Parameter types after making them visible is not supported, I tried generating errors when a Parameters type is changed, but reverted the corresponding commit with a force push, since that generated errors in the example relativity_demo.py.

For the other parameters:

  • strictNaming, readonly: From my tests, changes in that Parameter option are already reflected in UI.
  • enabled: From my tests, this option does not seem to do anything, so no change has to be reflected.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 30, 2020

@2xB I merged another PR a bit ago, which appears to have caused some conflicts with your branch. Would you mind resolving those, which should trigger another CI run (which I was about to do anyway).

Thanks.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 30, 2020

@2xB I accidentally merged a PR that broke the test_pg_edit ...I'm working on a PR to skip that test on pyqt 5.9 right now.

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented May 30, 2020

@j9ac9k Isn't that the test_pg_exit test that is failing randomly on macOS Py3.6 PyQt5.9 since quite a while now and that you just managed to replicate locally, as you reported in #1163? I don't think that is new since a specific recent PR ^^

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 30, 2020

That test_pg_exit test started failing on a variety of PRs, but not all of them. I was doing my best last night to merge PRs that were not affected, but either I merged one that was impacted, or the combination of a PRs that I merged, where each passed on their own caused this failure. I am able to reproduce locally, but I suspect this is a Qt specific issue, not a pyqtgraph one, so decided to just skip this test on Qt 5.9 builds (not just macOS).

Anyway, now that the dev build is failing this test, now your PR is going to show that test failing regardless of your code change (until I merge #1215)

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented May 30, 2020

Retriggering CI

@2xB 2xB closed this May 30, 2020
@2xB 2xB reopened this May 30, 2020
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 30, 2020

Green across the board 🎊 Merging, thanks for the PR @2xB

@j9ac9k j9ac9k merged commit 7672b5b into pyqtgraph:develop May 30, 2020
2xB added a commit to 2xB/pyqtgraph that referenced this pull request Jun 6, 2020
This issue was introduced in merging develop into pyqtgraph#1175.
While refactoring for the merge, the change in namespace was not
correctly attributed, leading to the parameter `opts` to be assumed
in local namespace when it isn't.
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.

ParameterTree, GroupParameter, saveState() and loadState() do not work / do not exist

3 participants