Fix issues due to the Parameterized private namespace #790
+192
−30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maximlt
commented
Jul 12, 2023
philippjfr
reviewed
Jul 14, 2023
|
This is again ready for review @philippjfr |
philippjfr
approved these changes
Jul 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#766 was merged too quickly, running the HoloViews test suite locally revealed a bunch of new errors:
__init__before callingsuper().__init__wasn't exerted in Param's test suite but leveraged in HoloViews (https://github.com/holoviz/holoviews/blob/a64cfc544acc94b3806d427912047ca660253587/holoviews/plotting/plot.py#L1168). This is now supported again and tested.0is used which is apparently used by HoloViews in some cases. This PR fixes the support for that protocol, at least in the sense that the HoloViews unit test suite should now pass. To complete that fix, I have had to remove the compatibility code onWatcherthat allowed to excludeprecedencewhile unpacking (added in Add compatibility layer for Param watcher precedence #557). I believe that compatibility code is no longer required for Panel. The code in Param was released in 1.12.0 and Panel 0.13.0, which depended on Param >= 1.12.0, was released with updated code that no longer required the compatibility code (Compatibility for latest Param panel#2809). I had to remove that code as, I think,__iter__was called both when pickling and unpickling, resulting in the last two attributes being filtered out (error I got:TypeError: __new__() missing 1 required positional argument: 'queued'). I would hope no other Param dependents relied on that compatibility code like Panel used to do, and if they did I don't feel too bad breaking that for Param 2.0.Working on these fixes I have stumbled upon this PR #400 which had no tests, I have added some (thinking about another WIP PR that touches constant/instantiate too)