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

Bump isort, enable Cython package resorting #806

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

charlesbluca
Copy link
Member

@charlesbluca charlesbluca commented Jun 14, 2021

With rapidsai/integration#286, the version of isort running on gpuCI will be bumped to 5.6.4, allowing us to enforce the sorting of packages in Cython (pyx, pxd) files. This PR intends to:

  • Enable these checks in the gpuCI style script
  • Enable Cython package resorting in the pre-commit hook
  • Resort all the Cython files in this repo so they pass the newly enabled checks

@charlesbluca charlesbluca requested a review from a team as a code owner June 14, 2021 20:42
@github-actions github-actions bot added the gpuCI label Jun 14, 2021
@jakirkham jakirkham added non-breaking Non-breaking change Python Related to RMM Python API feature request New feature or request labels Jun 14, 2021
@harrism harrism added this to PR-WIP in v21.08 Release via automation Jun 15, 2021
v21.08 Release automation moved this from PR-WIP to PR-Reviewer approved Jun 24, 2021
@harrism
Copy link
Member

harrism commented Jun 24, 2021

Does this PR depend on rapidsai/integration#286 ?

@dillon-cullinan
Copy link
Contributor

Ya, should hold off on this.

@charlesbluca
Copy link
Member Author

I think it's the other way around - shouldn't we merge this in first, so that any subsequent PRs have the properly sorted Cython files that rapidsai/integration#286 enforces? See this comment

@dillon-cullinan
Copy link
Contributor

Ah, my mistake. I thought you were adding functionality that doesn't exist in the current version of isort. This is safe to merge.

@harrism
Copy link
Member

harrism commented Jun 24, 2021

Thanks for clarifying!

@harrism
Copy link
Member

harrism commented Jun 24, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f7884b3 into rapidsai:branch-21.08 Jun 24, 2021
v21.08 Release automation moved this from PR-Reviewer approved to Done Jun 24, 2021
@charlesbluca
Copy link
Member Author

Hmm, my bad - I just noticed that there were some PXD resorts that should've been merged in this that will probably cause CI failures once rapidsai/integration#286 is merged. I will open a follow up PR that applies those changes.

@charlesbluca
Copy link
Member Author

charlesbluca commented Jun 24, 2021

Opened up #812 to address this

rapids-bot bot pushed a commit that referenced this pull request Jun 24, 2021
I neglected to run the updated isort hooks from #806 before it was merged - this should ensure that rapidsai/integration#286 doesn't cause any CI failures here when it is merged.

Authors:
  - Charles Blackmon-Luca (https://github.com/charlesbluca)

Approvers:
  - https://github.com/jakirkham

URL: #812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request gpuCI non-breaking Non-breaking change Python Related to RMM Python API
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants