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

fix(python): Make with_columns in with_columns_kwargs mode compatible with more data types #6126

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

gab23r
Copy link
Contributor

@gab23r gab23r commented Jan 8, 2023

fixes #6104

I tried to make a TypeAlias for :

( pli.Expr | bool | int | float | str | pli.Series | None | date | datetime | time | timedelta | pli.WhenThen | pli.WhenThenThen | Sequence[(int | float | str | None)] )
I did not make it work with mypy, I get this error F722 Syntax error in forward annotation: which I did not really understand.

@github-actions github-actions bot added fix Bug fix python Related to Python Polars labels Jan 8, 2023
@gab23r
Copy link
Contributor Author

gab23r commented Jan 10, 2023

@alexander-beedie, this PR can go like this or need more ?

@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Jan 11, 2023

@alexander-beedie, this PR can go like this or need more ?

Hi @gab23r - did you see the review comment above? I'm suggesting you change the **named_kwargs type signature to Any, instead of the large list of specific types. With that change it should be good to go 👍

@gab23r
Copy link
Contributor Author

gab23r commented Jan 11, 2023

What ? No I don't see any review comment... 🤨
Anyway I will change that, thanks

@ritchie46
Copy link
Member

Let me know if it's good to go @alexander-beedie.

@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Jan 11, 2023

Let me know if it's good to go @alexander-beedie.

@ritchie46: go for it; I can tidy-up the type hint on my next commit - there's nothing wrong with it, it's just long ;)

@gab23r: thanks for the PR!

@ritchie46
Copy link
Member

Thanks all!

@ritchie46 ritchie46 merged commit 6951e06 into pola-rs:master Jan 11, 2023
@gab23r gab23r deleted the with_columns_kwargs_all_dtypes branch January 14, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistency using with_columns_kwargs
3 participants