Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: type hints for tab completion #2769

Closed
wants to merge 20 commits into from
Closed

WIP: type hints for tab completion #2769

wants to merge 20 commits into from

Conversation

ChristopherDavisUCI
Copy link
Contributor

This is an attempt to have tab completion for the property setters by using type hints, as in this comment. I've never worked with type hints before, so I don't know if there is any issue with this approach.

@joelostblom and @mattijn could you please take a look and let me know your impressions?

@mattijn
Copy link
Contributor

mattijn commented Dec 22, 2022

Hi @ChristopherDavisUCI, seems you've made great progress!

I tried the following in jupyterlab:

import altair as alt

alt.X().axis(None).title(None)

I was only able to tab-complete the first encoding channel option, here axis(), and not for the following chained encoding channel options, here title(). Can you confirm if this also happens on your side?

@joelostblom
Copy link
Contributor

@ChristopherDavisUCI Great! It might take some time for me before looking closer at this, sorry about that.

@mattijn What you describe sounds like it could be related to this issue in jupyterlab jupyterlab/jupyterlab#12570

@ChristopherDavisUCI
Copy link
Contributor Author

@mattijn @joelostblom Thanks for checking it out! @mattijn yes I can confirm that doesn't work on either Jupyter notebook or Jupyter lab for me. I wasn't careful about specifying the return type of these methods, so maybe that will help.

@mattijn
Copy link
Contributor

mattijn commented Dec 22, 2022

Each encoding channel option should return the encoding channel class in which it is defined, isn't it? Is this possible with these generic protocol class objects?

@ChristopherDavisUCI
Copy link
Contributor Author

Closing in favor of #2770 which addresses the issues raised above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants