Skip to content

fixed recursive PlotWidget.__getattr__ calls#2860

Merged
j9ac9k merged 3 commits intopyqtgraph:masterfrom
nleclercq:recursive_getattr_fix
Oct 29, 2023
Merged

fixed recursive PlotWidget.__getattr__ calls#2860
j9ac9k merged 3 commits intopyqtgraph:masterfrom
nleclercq:recursive_getattr_fix

Conversation

@nleclercq
Copy link
Copy Markdown
Contributor

PlotWidget.__getattr__ implicitly wrap methods from PlotItem. However, the current implementation leads to recursive calls
and 'maximum recursion depth exceeded' errors when PlotWidget is used in a multiple inheritance schema - i.e., class SomeClass(PlotWidget, SomeOtherClass). The proposed PlotWidget.__getattr__ prevents the recursive calls by get rid of the hasattr that is the source of the problem.

m = getattr(self.plotItem, attr)
if hasattr(m, '__call__'):
return m
except:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you specify which exception to catch here instead of the bare exception?

Copy link
Copy Markdown
Contributor Author

@nleclercq nleclercq Oct 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in fact, no need to modify the implementation of PlotWidget.__getattr__. Moreover, the problem has nothing to do with multiple inheritance. It comes from the fact that the PlotWidget.__getattr__ tries to access the PlotItem attribute before it actually exists - generating recursive calls to PlotWidget.__getattr__ while trying to access self.PlotItem.
The simplest solution is to instantiate the PlotItem attribute at the very beginning of the PlotWidget.__init__ and we are done withe the problem.

def __init__ (self, parent=None, background='default', plotItem=None, **kargs):
   ## start by instantiating the plotItem attribute in order to avoid recursive 
   ## calls to PlotWidget.__getattr__ - which accesses self.plotItem!
   self.plotItem = None
   ...

PR modified.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 29, 2023

Love the modified PR, creating the attribute at initialization is a good way to go; not sure why we just don't instantiate the PlotItem instance at __init__ but I feel like that might have other consequences outside the scope of this PR; so if this fixes your issue, then leave as is 👍🏻

@j9ac9k j9ac9k merged commit 9824a15 into pyqtgraph:master Oct 29, 2023
@nleclercq
Copy link
Copy Markdown
Contributor Author

BTW, when do you plan to publish a new release?

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 29, 2023 via email

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.

2 participants