Miscellaneous fixes to opts validation #3257
Conversation
holoviews/util/__init__.py
Outdated
| if invalid: | ||
| try: | ||
| cls._options_error(list(invalid)[0], element, | ||
| Store.current_backend, allowed) | ||
| cls._options_error(list(invalid)[0], element, None, allowed) |
There was a problem hiding this comment.
Does this support passing an explicit backend kwarg? The way it works for .options is that if you pass in a specific backend it becomes completely strict otherwise it's permissive about options which apply to one backend but not another. So if we want to support that you'd pop backend from kws above and then replace None here with the popped backend (defaulting to None).
There was a problem hiding this comment.
Initially I thought it would be trivial to support this but in the end I needed to do a little refactoring in b43f60e to get this working.
|
@philippjfr I renamed |
holoviews/core/dimension.py
Outdated
| kwargs['clone'] = clone | ||
|
|
||
| if apply_option_types and util.config.future_deprecations: | ||
| param.main.warning("Dictionary signature of opts method deprecated. " |
There was a problem hiding this comment.
This warning does not seem particularly informative, what dictionary format? I'd probably be specific about it and have very specific warnings for each of the three conditions above since I don't think there is a clear way of describing all the nested formats at once.
There was a problem hiding this comment.
Hmm, I'm not sure that is really necessary - the warning message points to hv.opts.apply_option_types (not sure about that name!) which would accept whatever signature was supplied with only very minor modification (obj as the first argument). The signature is also described better on the docstring of that method.
In other words, I would expect the only people to see this warning would be people using .opts the old way in which case they should start using hv.opts.apply_option_types and I don't really see why the warning itself needs to go into the details about the signature...
There was a problem hiding this comment.
Okay, fair enough, maybe just clarify "dictionary signature" a bit more, something like:
Calling the .opts method with options broken down by options group (i.e. separate plot, style and norm groups) is deprecated. Use the .options method converting to the simplified format instead or use hv.opts.apply_option_types for backward compatibility.
|
@philippjfr Ready to review/merge once the tests are green. |
holoviews/util/__init__.py
Outdated
| @@ -95,6 +95,94 @@ def __call__(self, *args, **params): | |||
|
|
|||
| self._cellmagic(args[0], args[1]) | |||
|
|
|||
|
|
|||
| @classmethod | |||
| def option_groups(cls, obj, options=None, backend=None, clone=True, **kwargs): | |||
There was a problem hiding this comment.
Didn't want to call this apply_option_groups or similar?
There was a problem hiding this comment.
Ah yes, apply_groups I guess!
There was a problem hiding this comment.
Fixed in fb81455, thanks.
holoviews/core/dimension.py
Outdated
|
|
||
| For backwards compatibility, this method also supports the | ||
| option group semantics now offered by the | ||
| hv.opts.apply_options_type utility. This usage will be |
There was a problem hiding this comment.
Called apply_options_type here?
|
I haven't looked but aren't there changes needed for subclasses that define an |
I'll double check that, but I think they all delegate most of the actual work via Edit: You are right, if nothing else the signatures need to be made consistent. |
jlstevens
force-pushed
the
opts_validation_fix
branch
from
d48141d
to
fb81455
Compare
jlstevens
force-pushed
the
opts_validation_fix
branch
from
cc663c6
to
bad192b
Compare
Fixes
opts.defaultsand enables cross-backend validation in thehv.optscompleters.