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

remove thriftpy2 ceiling #4521

Merged
merged 5 commits into from
Jul 19, 2024
Merged

Conversation

jameslamb
Copy link
Member

#4496 introduced a ceiling on thriftpy2. Context: #4496 (comment)

The bug that ceiling was added to avoid was fixed in v0.5.2 of thriftpy2, which was just released (#4478 (comment)).

This removes that, adding != constraints to skip the 2 versions that cugraph was not compatible with.

Notes for Reviewers

Why not a floor?

I'm proposing adding != constraints to skip v0.5.0 and v0.5.1 to maximize cugraph's compatibility with other environments... that'd allow it to be used in environments with thriftpy2<0.5.0 and in environments with thriftpy2>0.5.2.

Let me know if you'd prefer the simplicity of a floor like >=0.5.2 instead.

@jameslamb
Copy link
Member Author

jameslamb commented Jul 5, 2024

This is blocked until #4522 is merged.

@jameslamb jameslamb changed the title WIP: remove thriftpy2 ceiling remove thriftpy2 ceiling Jul 8, 2024
@jameslamb jameslamb marked this pull request as ready for review July 8, 2024 19:17
@jameslamb jameslamb requested a review from a team as a code owner July 8, 2024 19:17
@jameslamb
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit c9d2843 into rapidsai:branch-24.08 Jul 19, 2024
136 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conda improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants