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

types are referenced from too deep #3335

Closed
mattijn opened this issue Feb 21, 2024 · 3 comments
Closed

types are referenced from too deep #3335

mattijn opened this issue Feb 21, 2024 · 3 comments

Comments

@mattijn
Copy link
Contributor

mattijn commented Feb 21, 2024

Given the following spec:

import altair as alt
from vega_datasets import data

cars = data.cars()
alt.Chart(cars).mark_point().encode(
    x='Horsepower',
    y='Miles_per_Gallon',
    color='Origin',
)

I created a screenshot of the documentation of alt.Chart (the one that appears when using shift+tab):

image

It gives type hints for the available options. That is nice! I underlined just one random, which is currently referred as

altair.vegalite.v5.schema.core.Generator

But almost all of these type hints are also available from the altair root, like:

alt.Generator

Can we change this reference for these type hints? This will make these information panels with documentation and type hints easier to understand.

As was discussed here: #3333 (comment).

Side note: some of these type hints are not available from the altair root, such as:

altair.utils.data.SupportsGeoInterface

Which is not available under

alt.SupportsGeoInterface

(edit: of which I think it should be available from the root)

@binste
Copy link
Contributor

binste commented Feb 21, 2024

Thanks for writing this down! Unfortunately, this might not be possible. A type is defined in one place and at least VS Code and Jupyter Lab use the location of the actual definition of the type which is also consistent with Python's type function. I did not find a way to change this.

Here's an example. We define class A in one file:

first_module.py:

class A:
    pass

import it in another:

another_module.py:

from first_module import A

And then from there import it in a notebook and use it as a type hint:

image

It shows as first_module.A although we imported it from another_module.

Now that Altair no longer supports different Vega-Lite and even Vega versions, I'd be in favour of simplifying the folder structure for the next major release when it comes. This would help with version control (all files in new v5, v6, ... folders show up as "new" which makes it difficult to compare against older versions) and also with type hints. It could simply be altair.schema.core.Generator.

@mattijn
Copy link
Contributor Author

mattijn commented Feb 21, 2024

Hm that is unfortunate. Yes, I agree that simplifying the folder structure with a major release is good alternative solution to improve this.
As the current folder structure has become very visible with the type hints..

@binste
Copy link
Contributor

binste commented Feb 24, 2024

I've opened #3337 to track the idea of simplifying the package structure.

@binste binste closed this as completed Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants