-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
fixed issue 1581 #1584
fixed issue 1581 #1584
Conversation
@philippjfr . Let me know if there is more I should do. |
Codecov Report
@@ Coverage Diff @@
## master #1584 +/- ##
==========================================
- Coverage 85.68% 85.68% -0.01%
==========================================
Files 147 147
Lines 16414 16440 +26
==========================================
+ Hits 14064 14086 +22
- Misses 2350 2354 +4
Continue to review full report at Codecov.
|
panel/param.py
Outdated
@@ -227,7 +227,8 @@ def _update_widgets(self, *events): | |||
parameters = [] if event.new == [] else event.new | |||
|
|||
if parameters != [] and parameters != self.parameters: | |||
self.parameters = parameters | |||
if not self.parameters: | |||
self.parameters = parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change does not seem correct to me, but I can see how the previous logic was incorrect too. Will keep your test though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
Okay, added what I think is the correct fix. That said I also can't explain the |
Okay I understand it now and left comments. The |
9512ee3
to
0e4e69a
Compare
* fixed issue 1581 * Handle condition where Param.parameters was set * Keep track whether parameters was explicitly set Co-authored-by: Marc Skov Madsen <masma@orsted.dk> Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
This should fix #1581
I've added a test and a one line fix. For me all tests pass.
But I simply don't understand the logic of the function I've changed. Are we sure that all combinations of events wil work?