-
Notifications
You must be signed in to change notification settings - Fork 337
[FEA] Update pylibcugraph to support different temporal comparisons #5345
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
[FEA] Update pylibcugraph to support different temporal comparisons #5345
Conversation
rlratzel
left a comment
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.
Can you add at least one test that uses the new arguments?
Also, is using strings >, <, etc. as the arg value a common convention that is already familiar to callers? Otherwise, it seems like it would be much more self-documenting to take strings like "strictly_increasing", "monotonically_decreasing", etc. (or some abbreviation of those).
| @@ -1,4 +1,4 @@ | |||
| # SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION. | |||
| # SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION. | |||
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.
I don't think this is supposed to be indented?
|
I think that's fair, I'll add a test and change the argument names. |
rlratzel
left a comment
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.
Thanks
|
/merge |
2 similar comments
|
/merge |
|
/merge |
Updates pylibcugraph to support different temporal comparisons (i.e. >=, <, <=, >, last).