-
Notifications
You must be signed in to change notification settings - Fork 908
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
Skeleton cudf polars package #15688
Skeleton cudf polars package #15688
Conversation
0f32757
to
8ec9471
Compare
a37cdbb
to
9d5ae66
Compare
# flake8-pytest-style: | ||
"PT011", # pytest.raises({exception}) is too broad, set the match parameter or use a more specific exception | ||
# flake8-simplify | ||
"SIM108", # Use ternary operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you not like the ternary in some cases? Could you give me an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a disabled lint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is disabling a suggestion to switch from a multi-line if/else to a ternary, right? Isn't that something we would normally like, if it does improve brevity?
I don't feel strongly, just asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah true. I suppose it's a stylistic choice rather than "you must do one or the other". So I'd rather leave it: some single-statement assignments can be very long when written in ternary form, but clear if as if/else blocks, and I'm not sure how ruff decides when to recommend one over the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it comes time to actually build something here, I think you'll need to add a devcontainers manifest.yaml entry, like rapidsai/devcontainers#271
/merge |
Description
Introduce the skeleton of a cudf_polars package. Note that we are deliberately not building any packages yet.
Checklist