Limit recursion in reprs to avoid stack overflow for circular references

[MarcSkovMadsen]

System Info

• param 1.9.3
• pytest 5.3.1
• Windows 8.1

My Pain

I'm developing a component based on Param and Panel and would like to test it with pytest. During development my tests stopped working/ running but without errors. After some debugging I discovered it was due to a stackoverflow that pytest did not report. It just stopped.

I've developed a smaller code example below. I've also included the stack trace.

Stack Trace

$ python 'test_issue_multple_param_action.py'
Fatal Python error: Cannot recover from stack overflow.

Current thread 0x00001be8 (most recent call first):
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 1535 in get_param_values
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 2408 in __repr__
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 2408 in <listcomp>

..........

  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 2408 in <listcomp>
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 2408 in __repr__
  ...
(.venv)

Code

import param

class InputDataComponent(param.Parameterized):
    update_data = param.Action()
    download_data = param.Action()

    def __init__(self, **params):
        super().__init__(**params)

        self.update_data = self._update_data
        self.download_data = self._download_data

    def _update_data(self, event):
        raise NotImplementedError()

    def _download_data(self, event):
        raise NotImplementedError()

repr(InputDataComponent())

[MarcSkovMadsen]

Additional Context

It's no problem with only one param.Action

$ python 'test_issue_multple_param_action.py'
(.venv)

Code

import param

class InputDataComponent(param.Parameterized):
    update_data = param.Action()
    # download_data = param.Action()

    def __init__(self, **params):
        super().__init__(**params)

        self.update_data = self._update_data
        # self.download_data = self._download_data

    def _update_data(self, event):
        raise NotImplementedError()

    def _download_data(self, event):
        raise NotImplementedError()

repr(InputDataComponent())

[MarcSkovMadsen]

Workaround

Setting the __repr__ to something specific works around the problem.

Thanks to @jbednar

$ python 'test_issue_multple_param_action.py'
(.venv)

import param

class InputDataComponent(param.Parameterized):
    update_data = param.Action()
    download_data = param.Action()

    def __init__(self, **params):
        super().__init__(**params)

        self.update_data = self._update_data
        # self.download_data = self._download_data

    def _update_data(self, event):
        raise NotImplementedError()

    def _download_data(self, event):
        raise NotImplementedError()

    def __repr__(self):
        return f"InputDataComponent(name='{self.name}')"

repr(InputDataComponent())

[ceball]

xref #209

[ceball]

@MarcSkovMadsen sorry, unrelated question! When I click to open a new issue on a holoviz repo (including param), I get the view below, and then the issue is created with labels like "triage" and "bug" (depending which option I pick):

Meanwhile, your issue is somehow without that stuff.

We've been trying to improve all this stuff across holoviz, having org-wide templates and so forth - but seems like we haven't done something right somewhere...

[jbednar]

Ok, this issue should now be about a request to add proper limits on the levels of recursion to which we expand Parameterized objects in reprs.

I vote for two levels, so that printing a Parameterized prints its parameter values (which may themselves be Parameters), but does not print the parameter values of the Parameterized subobjects: Component1(a=Component2(...), b=Component2(...)).

Three levels is a bit more informative but to me seems likely to be more overwhelming than useful (though it's not a strong preference for 2): Component1(a=Component2(x=Component3(...)), b=Component2(x=Component3(...))).

Beyond that seems like overkill to me...

[philippjfr]

The real problem seems to be infinite recursion, i.e. two Parameterizeds that are nested and reference each other. Ideally we should make sure that a Parameterized should not appear more then once in the repr and indicate the recursion through some abbreviation. Not saying we shouldn't also limit the depth of the repr but it doesn't seem like the real root of the issue.

[jbednar]

Sure, the serious problem is infinite recursion, but it seems like we can easily avoid it rather than solving it. Solving it would seem to require keeping a list of id values that have been presented so far and ensuring that we never print it the second time. That seems like a lot more work than simply always limiting how deep we display, which shouldn't require any tracking of ids or other special mechanisms. E.g. for two-level reprs we'd simply iterate through our parameters and call repr with a value saying not to show sub-parameters, which I think would be trivial to implement and has the advantage of simplifying our reprs in general (unless people find that representation impoverished).

[philippjfr]

. That seems like a lot more work than simply always limiting how deep we display, which shouldn't require any tracking of ids or other special mechanisms. E.g. for two-level reprs we'd simply iterate through our parameters and call repr with a value saying not to show sub-parameters

Doesn't seem at all straightforward to me, the simple case of a Parameter that holds a Parameterized is of course, but even in this example that wasn't the issue. Here the param.Action points to a method on the class, so now we have to inspect methods as well to be able to pass the depth through. However then we still need to handle parameterizeds that are nested inside some container, i.e. list or dictionary. I think only a global variable which stores the current repr depth will work, which also isn't the nicest.

[jbednar]

Good point. Yes, that's annoying.

[jbednar]

It looks like the repr in this case should be:

InputDataComponent(download_data=<bound method 
    InputDataComponent._download_data of ...>, name='InputDataComponent00001', 
    update_data=<bound method InputDataComponent._update_data of ...>)"

(where ... refers to itself). I don't suppose there's any way to control the repr of a bound method. I guess Parameterized's repr could check for a value that's a bound method of self, but that wouldn't catch a reference to something that points back to this object.

[MarcSkovMadsen]

Just for info. I just experienced this again in another use case. I can live with it now that I know the cause and a workaround. But I think it could be annoying for a lot of other users.

[MarcSkovMadsen]

Justed wasted 30 minutes on debugging strange stack overflow when testing with Pytest before I released it was due to this again.

I get no indication that this is the error. But happens often when I have two or more param.Action on my param.Parameterized class.

[philippjfr]

We should try this https://twitter.com/raymondh/status/1276571193710047232?s=19

[MarcSkovMadsen]

@mccav experiences this problem. See https://discourse.holoviz.org/t/how-do-i-use-param-action-in-panel/923/5?u=marc

and also this https://discourse.holoviz.org/t/how-do-i-use-param-action-in-panel/923/6?u=marc

[jlstevens]

This is another reason to drop Python 2 soon (and use that decorator, assuming it works for our purposes).

[jbednar]

Fixed in #499.