Type hints: Simplify to improve user experiences

[binste]

With this PR I'm trying to address the feedback raised in #3290. It relaxes some of the type hints to improve the readability of the type hints in an IDE such as VS Code and Jupyterlab as well as in the docstrings:

• Instead of showing all specific Altair classes, use SchemaBase from which all autogenerated classes inherit from anyway. As an example, in the definition of Color.aggregate, we could replace Aggregate, ArgmaxDef, ArgminDef, NonArgAggregateOp, and the different Dict[required[argmax]]... etc. with just dict and SchemaBase. Same in docstrings.

Old:

New:

• Fix deduplication of type hints and flatten out nested type hints to one level, i.e. we no longer have Unions within other Unions.
• Remove unnecessary class name + dict from beginning of docstrings:

Old:

New:

[binste]

The latest commit adds back the full list of class names (instead of SchemaBase) in the docstrings based on the input from @joelostblom here

[joelostblom]

Thank you for working on this @binste ! I think your changes here makes the docstrings easier to read overall. A few comments:

Fix deduplication of type hints and flatten out nested type hints to one level, i.e. we no longer have Unions within other Unions.

Nice! I noticed that Literals are now also combined which I think is helpful as well:

However, it seems like they are not combined in the docstring (if this is linked to that we expand SchemaBase in the docstring, then let's just leave it as is):

For some reason Literal don't seem to be combined in JupyterLab, but I'm guessing that is a jlab issue and nothing we can improve here:

One question regarding the first VSCode screenshot above, I noticed that we have the type Sequence repeated several times. Is that something we can change on our end so that it says Sequence[str | bool | float | dict | SchemaBase] instead?

---

Somewhat related, I noticed that the docstring says that we accept Sequence[str] as the shorthand to e.g. alt.Color, is this correct? When I try with a list of strings

alt.Chart().mark_point().encode(alt.Color(['test:Q']))

I get this error:

SchemaValidationError: '[{'field': 'test', 'type': 'quantitative'}]' is an invalid value for `color`. Valid values are of type 'object'.

[joelostblom]

Another thing I noticed is that the signature to the mark_ methods does not include the Literal for the color options (but it includes Literal for other options such as the cursor):

whereas the docstring for MarkDef does:

I honestly don't think that's something we need to change, just something I noticed when reading through the help popups.

[binste]

Thanks for the review Joel! Here some first feedback:

Literals

In the code, the literals are not combined so seems like VS Code is doing this by itself. I'm somewhat indifferent if its easier to read if they are combined or not. If they are not combined, it gives a certain grouping but it's also more verbose. In any case, combining Literals in the type hints itself would require some larger rewriting of the code generation. If that's ok, I'd prefer to not tackle this in the current PR but happy to work on a follow up issue if you have a strong preference! :) Basically, when I'm flattening the type hints, a literal is already represented as a string, ready for code generation, e.g. 'Literal["ascending", "descending"]. I'd need to change that to keep the meaning of Literal and it's content "ascending", "descending" separat.

Sequence

Same story as with literals.

[binste]

Sequence[str] for shorthand

Struggling to remember the story here. In channels.py around line 40, there is some code which processes lists and tuples. This is done for all channels which is probably why I added the type hints.However, seems like this wouldn't produce a valid spec for all channels as you've shown for color. It works for tooltip where you can do: alt.Tooltip(["column_1", "column_2"]). I opened #3308. Maybe we can infer from the VL schema when it is valid and when it isn't.

[binste]

Another thing I noticed is that the signature to the mark_ methods does not include the Literal for the color options (but it includes Literal for other options such as the cursor):

...

I honestly don't think that's something we need to change, just something I noticed when reading through the help popups.

Nice catch! Weird... It's the same for me but the type hints are there in the code:

Seems like an issue with vs code, maybe it struggles with the length of these type hints? ;)

[joelostblom]

Thanks for the explanations! No need to do any large re-write of the code for combining the Literals; let's keep it the way it is and leave it up to the IDEs to take care of that.

This looks good from my side of things, but maybe let's leave open a couple of days to see if anyone wants to chime in on the main idea of abbreviating the signatures to use SchemBase and then the full expansion only in the docstring.

[mattijn]

Its difficult to get this right. There is a balance between making human-users happy and providing meaningful type-related context. Its great that we try to improve this step by step.

[binste]

Thank you both! Merging :)