Reverting UndefinedLike?

[mattijn]

In #2681, we have merged the pull request that

UndefinedLike = Any
Undefined: UndefinedLike = UndefinedType()

But the result now is that the docsctrings looks like this:

I do not think we favor this behavior. Maybe #2670 was sufficient already?

Undefined: Any = UndefinedType()

Or will this make the docstrings look like: data: Undefined = Any, encoding: Undefined = Any, etc?

[joelostblom]

I agree that the current docstring with UndefinedLike = Undefined is not very readable and would be in favor of changing it. I am not sure about what is the best way to change it and what effect that has for IDEs using typehints.

[mattijn]

I tried the approach as suggested in #2670 in this branch: master...mattijn:altair:revert-undefinedlike, but the docstring will then look like this in google colab:

!python -m pip install git+https://github.com/mattijn/altair.git@revert-undefinedlike
import altair as alt
alt.Chart()

From a user perspective not yet really improving.

Sorry to bother, but do you have some suggestions that might improve this @brahn / @arogozhnikov?

[brahn]

@mattijn I agree it's not great! Though I think a somewhat odd docstring is more desirable than the typechecker rejecting every call, which is the problem that #2670 and #2681 address. (Let me know if you need an example of what that looks like in an IDE. It's not pretty :-) )

My sense is that the real underlying problem is the use of Undefined as a default arg value rather than the more conventional choice of None. And so I suspect the best solution short of "real" typehints would be to replace all the Undefined default arg values with None.

[arogozhnikov]

surprised that colab shows :Any in hints as that gives no useful information (just checked that pycharm hides Any from hints).

Otherwise, that's a correct inferred signature, and I don't see a problem with it 🤷

[brahn]

I agree with @arogozhnikov that , though not pretty, the following is on accurate and more helpful than not.

[mattijn]

Thanks for the helpful feedback, let me turn the Undefined: Any = UndefinedType() into a PR then.

[joelostblom]

My sense is that the real underlying problem is the use of Undefined as a default arg value rather than the more conventional choice of None. And so I suspect the best solution short of "real" typehints would be to replace all the Undefined default arg values with None.

I agree with this. Seeing None is more natural in Python so maybe we should consider a "replace all" as part of the auto docstring generation? Or would there be any issues from this?

[brahn]

@joelostblom Not 100% sure I follow -- are you proposing...

• A. Replacing the Undefined default arg values with None or
• B. Modifying the doc string to make it look like the default arg values are None?

If A is doable that sounds ideal! B sounds undesirable since the docstring would be prettier but is disagreeing with "the truth" about the code, and I worry that would be even more baffling.

If the goal is to improve readability of the docstring I'd suggest instead adding linebreaks between the args and maybe a note explaining that Undefined is a special "no value" type defined by altair.

[mattijn]

Comment from long time ago #364:

all traitlets objects are now based on the JSONTraitlets extensions provided by schemapi. One big reason for this is that the default value is now Undefined, and a value of None will be interpreted as an explicit null in the JSON output.

In my understanding this means that the default argument can not be None. But correct me if I'm wrong here.

I think I favor Any to solve these pylance issues for now and if we do not like the styling of the docstrings then fix that following the above mentioned suggestions.

Improving typings to a better level might already be in #2614.

[joelostblom]

@brahn I was referring to A and I meant modifying the function signature, not the docstring as I wrote initially, sorry about the confusion caused.

@mattijn Nice job digging that up. I agree with you that a solution to fix the issues for now that we can later iterate on if we want to improve the docstring styling is a good idea.

Also, is this only an issue in Google Collab? I just had a look in JupyterLab with the latest master and the docstrings look great, very readable with one parameter per line. I don't see the typehints though so maybe jupyterlab does not support that yet?

Although is seems like it does for pandas:

[mattijn]

Thanks again for the comments here! Let's hope to have proper type support in the future.

Moreover, if people have a strong knowledgeable opinion on this, #2614 is a good place to discuss this further.