Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

param.pprint breaks if HoloViews is imported #785

Closed
hoxbro opened this issue Jul 4, 2023 · 5 comments · Fixed by #786
Closed

param.pprint breaks if HoloViews is imported #785

hoxbro opened this issue Jul 4, 2023 · 5 comments · Fixed by #786
Labels
type-bug Bug report
Milestone

Comments

@hoxbro
Copy link
Member

hoxbro commented Jul 4, 2023

Running the following:

import holoviews as hv
import param


class T(param.Parameterized):
    a = param.Number(1)


t = T()
t.param.pprint()

Will give the following traceback:

Traceback (most recent call last):
  File "/home/shh/Development/holoviz/development/tmp.py", line 10, in <module>
    t.param.pprint()
  File "/home/shh/Repos/holoviz/param/param/parameterized.py", line 2894, in pprint
    return partial(self_._pprint, self, imports=imports, prefix=prefix,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/shh/Repos/holoviz/param/param/_utils.py", line 105, in wrapper
    result = user_function(self, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/shh/Repos/holoviz/param/param/parameterized.py", line 2914, in _pprint
    args = spec.args[1:] if spec.args[0] == 'self' else spec.args
                            ~~~~~~~~~^^^
IndexError: list index out of range

If I add __init__ to the class, it works or remove the holoviews import it works again.

class T(param.Parameterized):
    a = param.Number(1)
    
    def __init__(self):
        super().__init__()
@hoxbro hoxbro added the TRIAGE User-submitted issue that hasn't been triaged yet. label Jul 4, 2023
@maximlt maximlt transferred this issue from holoviz/holoviews Jul 4, 2023
@maximlt maximlt added type-bug Bug report and removed TRIAGE User-submitted issue that hasn't been triaged yet. labels Jul 4, 2023
@maximlt maximlt added this to the 2.0 milestone Jul 4, 2023
@maximlt
Copy link
Member

maximlt commented Jul 4, 2023

It can be reproduced with:

import inspect
import param


class A(param.Parameterized): pass

A.__init__.__signature__ = inspect.Signature([inspect.Parameter('test', inspect.Parameter.KEYWORD_ONLY)])


class T(param.Parameterized):
    a = param.Number(1)


t = T()
print(t.param.pprint())

This is indeed what HoloViews is doing:
https://github.com/holoviz/holoviews/blob/a64cfc544acc94b3806d427912047ca660253587/holoviews/util/__init__.py#L626

But I don't understand yet why updating the docstring of a class (A) affects another one (T).

@hoxbro
Copy link
Member Author

hoxbro commented Jul 4, 2023

Isn't it because A's __init__ is param.parameterized.Parameterized.__init__
image

@maximlt
Copy link
Member

maximlt commented Jul 4, 2023

Yep indeed I also just figured this out. So I think there's two workaround?

  • param could just check that 'self' is in spec.args, that should be sufficient to avoid this breakage
  • HoloViews could add an __init__ method to its output ParameterizedFunction to avoid overriding the __signature__ of all the Parameterized objects :)

The real issue to fix has been well defined in #371.

@hoxbro
Copy link
Member Author

hoxbro commented Jul 4, 2023

I heavily favor number two. I think it is "correct" that params raise an error, as there is something "wrong" with what HoloViews does.

@maximlt
Copy link
Member

maximlt commented Jul 4, 2023

Ok I got slightly confused by inspect.signature that doesn't return the same output on the class or instance:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants