@j9ac9k nudging this PR... Any chance we can get someone to look at it? There are multiple aspects of the lib I can update to use interactive ptrees, but it is tedious to rework them multiple times. Once this is merged, substantially less effort required.

Note for anyone reviewing the changelist -- there is substantial associated documentation: https://pyqtgraph--2318.org.readthedocs.build/en/2318/parametertree/interactiveparameters.html

(There is also a "Parameter-function Interaction" example under python -m pyqtgraph.examples)

I am also happy to add to the docs to clarify any behavior

@ntjess you've had the patience of a saint on this code change, to say I appreciate your efforts here and patience is an understatement.

I looked this over, I have no code issues, I added a comment on some nit-pick stuff.

the other nit-pick comment I have, and this is not a mandate come here, this one I legit want to get your thoughts on is expanding out the use of Params to Parameters (same w/ the singular form) in some of the methods. I know we have methods like setOpts but one of the things I've been trying to do is not use too many abbreviations (other examples in the code are func instead of function, ch instead of child and so on).

Oh, the other ones that I have, on the RunOpts object, is ON_BUTTON the attribute we want, not ON_PRESS or ON_BUTTON_PRESS?

expanding out the use of Params to Parameters (same w/ the singular form) in some of the methods.

Good point, this is also how the tree API works, i.e. setParameters()

we have methods like setOpts

This was one of my reasons for using opts here as well, to be consistent with the existing Parameter conventions (kwargs are specified as **opts, the member attribute is self.opts, etc.). It would be difficult to deprecate at this point, otherwise I would agree with you. There are also multiple places where <*>opts are specified as keyword arguments, which is tedious to fully type everywhere so I'm inclined to lean toward the laziness efficiency 😅

func instead of function

The python standard library, sckikit-*, and other popular frameworks use it even if they don't abbreviate anything else, but Qt always expands their abbreviations. I'm happy to change it based on the goal of future code to avoid abbreviations where possible.

ch instead of child

This one I'm less concerned about, since it's only used inside the function and not part of the API. Qt also allows abbrevs (😉) within function blocks and pyqtgraph employs this often too. There are also so many instances of other non-API abbreviations that it would be difficult to refactor.

ON_PRESS or ON_BUTTON_PRESS?

I also considered something along these lines. ON_PRESS might be ambiguous, i.e. do you click the parameter group? ON_BUTTON_PRESS is a bit of a lengthy phrase (takes up 17% of an 88-char line length) compared to ON_BUTTON which I thought had clear enough connotations even if it's not a verb. I also considered ON_BTN_PRESS but that also introduces an abbreviation (albeit a common one). Thoughts?

(kwargs are specified as **opts, the member attribute is self.opts, etc.).

opts, kwargs, setOpts is used elsewhere in the library, I don't think we s should try and change those. func vs. functions and ch vs. child I leave up to you. We can not be in 100% agreement here and I would still merge this; PR author should have some discretion 😆

ON_PRESS or ON_BUTTON_PRESS?

I also considered something along these lines. ON_PRESS might be ambiguous, i.e. do you click the parameter group? ON_BUTTON_PRESS is a bit of a lengthy phrase (takes up 17% of an 88-char line length) compared to ON_BUTTON which I thought had clear enough connotations even if it's not a verb. I also considered ON_BTN_PRESS but that also introduces an abbreviation (albeit a common one). Thoughts?

For this one I think we might get some guidance/inspiration from the QAbstractButton signals:

https://doc.qt.io/qt-6/qabstractbutton.html#signals

That said, I don't think my comment there should hold up this PR either, I threw that out for your consideration 👍🏻

I've started reviewing this by basically just trying it out and building up a little demo.

One thing I've noticed so far is it doesn't seem possible to pass a class method to interact:

import pyqtgraph as pg
from pyqtgraph import parametertree as ptree


class Foo:
    def update(self, a=1):
        print(a)


app = pg.mkQApp()
foo = Foo()
params = ptree.interact(foo.update)
tree = ptree.ParameterTree()
tree.setParameters(params)
tree.show()
app.exec()

Output:

Traceback (most recent call last):
  File "/home/krlyons/src/pyqtgraph/interact_issue_method.py", line 12, in <module>
    params = ptree.interact(foo.update)
  File "/home/krlyons/src/pyqtgraph/pyqtgraph/parametertree/interactive.py", line 387, in __call__
    return self.interact(function, **kwargs)
  File "/home/krlyons/src/pyqtgraph/pyqtgraph/parametertree/interactive.py", line 335, in interact
    function = self._toInteractiveFunction(function)
  File "/home/krlyons/src/pyqtgraph/pyqtgraph/parametertree/interactive.py", line 456, in _toInteractiveFunction
    function.interactiveRefs = [interactive]
AttributeError: 'method' object has no attribute 'interactiveRefs'

It's not a huge hassle to write a wrapper function, but maybe this limitation could be mentioned in the docs so there's a search match if a user runs into it.

Great catch. TIL functions allow you to add member attributes, but methods don't. Without a stored reference to the InteractiveFunction, signals don't end up firing. In other words, you can get around this error by calling ptree.interact(InteractiveFunction(foo.update)) so no interactiveRefs variable is created, but the InteractiveFunction you create immediately gets gc'd and changed signals don't result in anything happening

There are a few ways around this:

1. As you suggested, update the docs to recommend the following procedure:

class MyClass:
    def func(self, a=1):
        print(a)
inst = MyClass()
# Or do this in __init__ depending on whether every instance should have this option
inst.interactive_func = InteractiveFunction(inst.func)

params = interact(inst.interactive_func)
# Same as your test code from here down

• As long as interactive_func is not gc'd, everything will work as normal. But if the user throws away this reference, the same problem arises that I mentioned earlier. That's why this example attaches it to the class instance

2. I could make InteractiveFunction a bit smarter so it recognizes when it is a class method, then this will work:

class MyClass:
    @InteractiveFunction
    def func(self, a=1):
        print(a)
cls = MyClass()
params = interact(func)
# Same as your test code from here down

• This would work in all cases, but adds a bit of overhead to calling func and requires subclasses to be aware of what's happening and also add their own decorators.

3. I can make interact return a reference to the InteractiveFunction, and users could leverage it as so:

inst.interactive_func, params = interact(inst.func)

• There can be a flag argument to interact for whether to return the instance to prevent this complexity. Again, it would work but at the cost of more complex behavior and the user is still responsible for properly maintaining the life cycle of the returned reference

4. (My tentative favorite) The failing line is func.interactiveRefs = .... Methods have a __func__ variable, which is a valid owner of the interactive ref and does allow setting attributes. Thus, if inspect.ismethod(func), I can attempt func.__func__.interactiveRefs = ... next.

What are your thoughts?

edit: Relevant changes to interactive.py:

refOwner = func.__func__ if inspect.ismethod(func) else func
# ^^^^^ This line
if hasattr(refOwner, "interactiveRefs"):
    refOwner.interactiveRefs.append(interactive)
else:
    refOwner.interactiveRefs = [interactive]

1 and 4 seem reasonable to me, also leaning toward 4. 2 and 3 would seem to add more complexity to usage where I think it's already a little complicated.

@ixjlyons sounds good. Can you take a look at the new test under tests/parametertree/test_Parameter.py for this scenario as well? There might be more edge cases.

Segmentation fault in CI? Uh-oh

In ROI test though? I don't think that's from this PR

@ntjess CI on this repo segfaults; even on re-run. On my fork, no segfault. Running it locally, no segfault...

Can you try pushing an empty commit to this branch and trigger a new CI run, I'm wondering if "re-run" is caching something problematic.

going to close/re-open to trigger a new CI Run, doing "re-run" doesn't seem to be cutting it.