altair
altair copied to clipboard
docstrings for new introduced methods regarding params and selection_*
Currently the docstrings for some new introduced parameter methods are rather limited:

Where in the above example I've defined it as such:
highlight = alt.selection_point(
name="hover",
on="mouseover",
nearest=True,
encodings=["x"],
empty=False,
clear="mouseout",
)
How can one know about this without docstrings?
I agree this should be more informative. I see that there are a couple of TODOs in that file on improving the docstrings for the parameters and for the bindings. Maybe we can use the same I approach I used #1629
https://github.com/altair-viz/altair/blob/91ddea9170e3949286daa9889f5e2bd91e3d8249/altair/utils/schemapi.py#L601-L604
to overwrite the docstring of selection_interval with that from the IntervalSelectionConfig class:
https://github.com/altair-viz/altair/blob/b186a76a25b6d7fd0c0efdeb58e0697c1876f813/altair/vegalite/v5/schema/core.py#L6329-L6349
On a related note, is there any use to keep having alt.selection() exposed as a public function when we mostly use alt.selection_point/interval directly? I don't think it offers anything valuable and just leads to ambiguity of which selection function to use, but I might be missing something. An alternative would be to only keep the former and always use alt.selection('point', ...) etc, but I think that is less common currently and in the docs (although more in line with VL).
Is it possible some of these issues have been incidentally fixed along the way? For example, I seem to have docstrings showing up okay.
shift+tab also seems to reveal a docstring (I can't get a screenshot at the moment).
A different issue is that these are the docstrings for the lower-level objects, such as core.IntervalSelectionConfig, so for example, name is not mentioned, even though name is an expected keyword argument for our higher-level Altair function.
Before looking into this further, I wanted to check @mattijn to what extent the issue you reported in October has been resolved. Thanks!
@joelostblom I agree it could make sense to hide alt.section, so that alt.selection_point and alt.selection_interval are encouraged. Do you remember how to do that?
You are right! This is fixed along the way. Great.

If you want to hide the alt.selection from the autocomplete, you'll have to assign alt.selection a deprecated status around here https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L372 by adding:
@utils.deprecation.deprecated(
message="'selection' is deprecated. Use xxx"
)
def selection(type=Undefined, **kwds):
Hm, not sure. This is the current docstring for alt.param, https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L303-L320.
def param(name=None, select=None, **kwds):
"""Create a named parameter.
Parameters
----------
name : string (optional)
The name of the parameter. If not specified, a unique name will be
created.
**kwds :
additional keywords will be used to construct a parameter. If 'select'
is among the keywords, then a SelectionParameter will be created.
Otherwise, a VariableParameter will be created.
Returns
-------
parameter: Parameter
The parameter object that can be used in chart creation.
"""
I would say that is too limited to consider this issue complete.
Glad the docstrings also working on your end @mattijn! The displayed signature isn't perfect, because on the Altair side we allow a parameter and its selection configuration to be defined simultaneously, but the above docstring only mentions the selection portion. For example, I believe all of these are missing from selection_interval: {"name", "value", "bind", "empty", "init", "views"}.
I agree that we should keep this issue open for now.
Hi @mattijn, I will try to improve the parameter docstrings. Is there anything wrong with just typing it out by hand in VS Code, including for example the valid keyword arguments? I just want to make sure that's reasonable before getting started.
Getting the valid arguments automatically from Vega-Lite is harder than usual for parameters, because on the Vega-Lite side, there is a parameter definition like this
{
"name": "brush",
"select": {"type": "interval", "encodings": ["x"]}
}
but on the Altair side, those values of "name" and "encodings" would typically be passed to the same function (like alt.selection_interval).
By hand is fine.
Is there a rule of thumb for when Undefined should be used as a default value, and when None should be used as a default value? I'm increasing the number of keyword arguments to alt.param (for example, bind and value), etc, but am finding myself just guessing whether to use None or Undefined as the default values.
In the context of Altair, None, serializes to null in JavaScript, sometimes that is desired. Again, in the context of Altair, you probably want Undefined most of the time.
Thanks @mattijn, please see if #2908 addresses this issue.