-
Notifications
You must be signed in to change notification settings - Fork 795
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
Passing basic polars dataframes to alt.Chart results in Pylance errors #3294
Comments
Have additionally confirmed this same exact error surfaces with a simple native Polar dataframe to rule out conversion-related issues: import polars as pl
import numpy as np
import altair as alt
df = pl.DataFrame(
{
"a": range(5),
"b": np.random.rand(5),
}
)
alt.Chart(df).mark_point().encode(
x="a",
y="b",
).interactive() Until this typing issue is fixed, all developers using Altair with Polars DataFrames will see this error and be forced to place |
I was just about to open an issue on this, been having the same problem since I updated to 5.2.0. Earlier this week I did some digging, and although I don't know enough about the Minimal repro import altair as alt
import polars as pl
def make_chart(chart: alt.Chart) -> alt.Chart:
return chart.encode(x="x", y="y").mark_point()
df: pl.DataFrame = pl.DataFrame({"x": [1, 1, 2, 3, 4], "y": [1, 2, 3, 4, 5]})
# all of these produce identical, valid charts
make_chart(alt.Chart(df)) # should be the only type check issue
make_chart(alt.Chart(df.to_arrow()))
make_chart(alt.Chart(df.to_pandas())) Notes
|
Thanks for the reports @christopherball and @dangotbanned We can probably fix this particular issue by changing to a Protocol definition as @dangotbanned suggested. But I just tried running pyright on our code base and it turns up hundreds of errors that mypy doesn't flag, so I'm not sure what to make of this. It may be difficult to test this change in isolations without a major effort to satisfy pyright. @binste do you have any thoughts here? |
Thanks for the report @christopherball and the very useful debugging information @dangotbanned! Fully agree, we should switch the definitions to Protocols instead of ABC as other libraries (of course) don't use subclasses of our ABCs. I'll submit a PR soon to fix this. Just switching to Protocols does not yet fully solve it, there is somehow some other difference in the protocols that Polars uses and that we use. I'll try to track it down. Btw, the issue can also be replicated with mypy. Regarding pyright errors on our code base, I think as mypy is the reference implementation for a static type checker, we should mainly aim to satisfy mypy. Pyright is in some ways more strict/follows different rules. In this case, it's in general a wrong type annotation and so we definitely want to fix it. |
I took the default boilerplate chart code from an altair example and simply converted the
cars
Pandas DataFrame to a Polars one (for testing purposes as I am using Polars), and while the code appeared to still execute, Pylance is catching some type issues. Restarted VSCode and still see the same problem:The text was updated successfully, but these errors were encountered: