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

Panel 1.3.0: ReactiveHTML with custom param.Parameter broken. #5730

Closed
emcd opened this issue Oct 24, 2023 · 11 comments · Fixed by holoviz/param#876
Closed

Panel 1.3.0: ReactiveHTML with custom param.Parameter broken. #5730

emcd opened this issue Oct 24, 2023 · 11 comments · Fixed by holoviz/param#876

Comments

@emcd
Copy link

emcd commented Oct 24, 2023

ALL software version info

(this library, plus any other relevant software, e.g. bokeh, python, notebook, OS, browser, etc)

$ python --version
Python 3.10.12
$ panel --version
1.3.0

Description of expected behavior and the observed behavior

Successful execution (like in Panel 1.2.3) is the expected behavior.
In Panel 1.3.0, the observed behavior is an AttributeError exception.

Complete, minimal, self-contained example code that reproduces the issue

from panel.reactive import ReactiveHTML
from param import Parameter

class Kaboom( ReactiveHTML ):

    foo = Parameter( )

    def __init__( self, **params ):
        self.foo = 1
        super( ).__init__( **params )

Kaboom( ).show( )

Stack traceback and/or browser JavaScript console output

$ python3 laboratory/panel-1.3.0-bug.py 
Traceback (most recent call last):
  File "/home/me/src/ai-experiments/laboratory/panel-1.3.0-bug.py", line 12, in <module>
    Kaboom( ).show( )
  File "/home/me/src/ai-experiments/laboratory/panel-1.3.0-bug.py", line 10, in __init__
    super( ).__init__( **params )
  File "/home/me/src/ai-experiments/.local/environments/langchain/lib/python3.10/site-packages/panel/reactive.py", line 1594, in __init__
    super().__init__(**params)
  File "/home/me/src/ai-experiments/.local/environments/langchain/lib/python3.10/site-packages/panel/reactive.py", line 545, in __init__
    if name not in self._param__private.explicit_no_refs:
AttributeError: '_InstancePrivate' object has no attribute 'explicit_no_refs'
@philippjfr
Copy link
Member

Can you report the param version?

@emcd
Copy link
Author

emcd commented Oct 24, 2023

@philippjfr : Looks like something is pulling in a release candidate rather than a stable release:

$ python3 -m pip list | grep param
param                     2.0.0rc7

I'll check to see which version is being pulled in under Panel 1.2.3. I just rebuilt my virtual environment a little while ago, so it's possible that another package is pulling in this dependency.

@philippjfr
Copy link
Member

Thanks, I can also reproduce.

@philippjfr
Copy link
Member

We should definitely fix this but you really shouldn't set a parameter value before calling super, i.e. this would be preferred:

from panel.reactive import ReactiveHTML
from param import Parameter

class Kaboom( ReactiveHTML ):

    foo = Parameter( )

    def __init__( self, **params ):
        params['foo'] = 1
        super( ).__init__( **params )

@philippjfr
Copy link
Member

Thankfully we haven't quite released Param 2.0 yet (you raised this just in time), so we can get a fix in before the release.

@emcd
Copy link
Author

emcd commented Oct 24, 2023

Ah, okay. I was not aware of that preference/limitation. Moving the super invocation to the top of the initializer does indeed provide a workaround. Feels a bit Python-non-intuitive that the behavior of setting an attribute on an object would be linked to superclass initialization. Thanks for the quick response.

@maximlt
Copy link
Member

maximlt commented Oct 24, 2023

For reference, there's a Param issue about deprecating this usage holoviz/param#815.

Feels a bit Python-non-intuitive that the behavior of setting an attribute on an object would be linked to superclass initialization.

Agreeing with you, Param can do better there.

@maximlt
Copy link
Member

maximlt commented Oct 24, 2023

@philippjfr the alternative to holoviz/param#876 is apply this change and release Panel 2.0.1. I assume you prefer patching Param before I release it today?

diff --git a/panel/reactive.py b/panel/reactive.py
index 83f3017ed..f1705b811 100644
--- a/panel/reactive.py
+++ b/panel/reactive.py
@@ -542,7 +542,7 @@ class Reactive(Syncable, Viewable):
 
     def __init__(self, refs=None, **params):
         for name, pobj in self.param.objects('existing').items():
-            if name not in self._param__private.explicit_no_refs:
+            if name not in type(self)._param__private.explicit_no_refs:
                 pobj.allow_refs = True
         if refs is not None:
             self._refs = refs

@philippjfr
Copy link
Member

Not sure, I wasn't actually aware that InstancePrivate and ClassPrivate differed so substantially. This does seem like a gotcha in general, especially since setting a parameter value will swap out the object. You might argue that only Param should access the private namespace but in practice there's probably a few places where Panel and maybe even users will want to access it.

@maximlt
Copy link
Member

maximlt commented Oct 24, 2023

I'd prefer Param to provide a public API here if there's a need for one. Let's patch Param for now and release, and then:

  • provide a public API for what you need in Panel in this case
  • update the internals of Param if they're too sketchy

emcd added a commit to emcd/ai-experiments that referenced this issue Oct 24, 2023
Also, fix issue related to the order of 'param.Parameterized' attribute
assignment and superclass initialization that was introduced in a
'param' package release candidate via the shiny new Panel 1.3.0 release.
holoviz/panel#5730
@philippjfr
Copy link
Member

Sounds fine to me.

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 a pull request may close this issue.

3 participants