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

Resolve non watched references too to catch bad references #777

Merged
merged 5 commits into from
Jul 11, 2023

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Jun 29, 2023

Fixes #357

@philippjfr I'm very far from being confident with this change.

This example:

import param

class A(param.Parameterized):
    x = param.Number(0)

    @param.depends('y', watch=True)
    def debug(self): pass

Would immediately fail at the class creating time with:

Traceback (most recent call last):
  File ".mltmess/issue_params.py", line 47, in <module>
    class A(param.Parameterized):
  File "/Users/mliquet/dev/param/param/parameterized.py", line 2903, in __init__
    deps, dynamic_deps = _params_depended_on(minfo, dynamic=False)
  File "/Users/mliquet/dev/param/param/parameterized.py", line 640, in _params_depended_on
    ddeps, ddynamic_deps = (minfo.cls if minfo.inst is None else minfo.inst).param._spec_to_obj(d, dynamic, intermediate)
  File "/Users/mliquet/dev/param/param/parameterized.py", line 2592, in _spec_to_obj
    raise AttributeError(f"Attribute {attr!r} could not be resolved on {src}.")
AttributeError: Attribute 'y' could not be resolved on <class '__main__.A'>.

@maximlt maximlt requested a review from philippjfr July 10, 2023 08:46
@philippjfr
Copy link
Member

My guess is that this breaks cases like this, which we didn't intend to support but did accidentally?

class A(param.Parameterized):

    def __init__(self):
        self.x = pn.widgets.FloatSlider()
        super().__init__()

    @param.depends('x.value')
    def update_on_x(self):
        return self.x.value * 2

@philippjfr
Copy link
Member

Nvm, still seems to work. Seems okay.

@maximlt
Copy link
Member Author

maximlt commented Jul 11, 2023

I added a test based on the comment above and ran the unit test suites of Panel and HoloViews with that branch checked out and saw no problem, I'm going to merge.

@maximlt maximlt merged commit 6692574 into main Jul 11, 2023
@maximlt maximlt deleted the resolve_non_watched branch July 11, 2023 16:14
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.

param.depends does not warn about unresolved parameters
2 participants