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

Update black version #1010

Merged
merged 6 commits into from
Apr 1, 2022
Merged

Update black version #1010

merged 6 commits into from
Apr 1, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Mar 28, 2022

This PR updates to black 22.3.0 which contains a critical bugfix for a recent release of Click 8.1.0. It also includes some minor reformatting associated with the version bump.

@vyasr vyasr added 3 - Ready for review Ready for review by team Python Related to RMM Python API non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Mar 28, 2022
@vyasr vyasr self-assigned this Mar 28, 2022
@vyasr vyasr added this to PR-WIP in v22.06 Release via automation Mar 28, 2022
@vyasr vyasr requested review from a team as code owners March 28, 2022 21:17
@github-actions github-actions bot added the gpuCI label Mar 28, 2022
@github-actions github-actions bot added the conda label Mar 28, 2022
@jakirkham
Copy link
Member

Thanks Vyas! 😄

Looks like we are getting some failures from black. Guessing this will be resolved by PR ( rapidsai/integration#452 )?

@vyasr
Copy link
Contributor Author

vyasr commented Mar 28, 2022

Yup, the version of black in the build environment won't actually be updated until the PR that you linked gets merged. Then the style checks here should pass.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 29, 2022

rerun tests

jjacobelli added a commit that referenced this pull request Mar 30, 2022
This is a backwards-compatible alternative to #1010 that does not require modifying linter behavior as we are in the middle of code freeze.

Authors:
   - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
   - https://github.com/jakirkham
   - GALI PREM SAGAR (https://github.com/galipremsagar)
   - Jordan Jacobelli (https://github.com/Ethyling)
@bdice
Copy link
Contributor

bdice commented Mar 30, 2022

Is this pulling the right version of black for the CI checks? (Is it sourced from the integration repo or the environment yml defined in this file?) I'll rerun tests just to check. CI output:

would reformat /workspace/python/rmm/rmm.py
would reformat /workspace/python/rmm/tests/test_rmm.py
Oh no! 💥 💔 💥
2 files would be reformatted, 11 files would be left unchanged.

@bdice
Copy link
Contributor

bdice commented Mar 30, 2022

rerun tests

@bdice
Copy link
Contributor

bdice commented Mar 30, 2022

This file needs to be updated as well: https://github.com/rapidsai/rmm/blob/branch-22.06/python/dev_requirements.txt

(Or maybe we just need to remove it? The conda environment has similar specifications. Also the dev requirements can be met entirely with pip/scikit-build + pre-commit now, I think, so the extra file may not be needed.)

@vyasr
Copy link
Contributor Author

vyasr commented Mar 30, 2022

Is this pulling the right version of black for the CI checks? (Is it sourced from the integration repo or the environment yml defined in this file?)

This should just be using whatever's in the integration repo, which is why tests were failing yesterday. However, now images are out and tests are still failing so I'm investigating further.

This file needs to be updated as well: https://github.com/rapidsai/rmm/blob/branch-22.06/python/dev_requirements.txt

(Or maybe we just need to remove it? The conda environment has similar specifications. Also the dev requirements can be met entirely with pip/scikit-build + pre-commit now, I think, so the extra file may not be needed.)

I'm not sure what to do with this. I believe it was originally added to support a Dockerfile that is no longer maintained within the repo. I'll just update it for now and we can address the potential cleanup in a subsequent PR.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 30, 2022

I've pulled the image locally and for some reason it's still got the wrong (old) version of black. My best guess is that the mixing of rapidsai/integration#452 and rapidsai/integration#455 caused the integration repo to upload slightly out-of-date images. I'll follow up with ops.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 30, 2022

It looks like the built images were indeed based on slightly out of date files. Once new versions are up we can run tests again.

@jakirkham
Copy link
Member

It seems the style check is failing related to black formatting. Are we still missing an update somewhere?

@vyasr
Copy link
Contributor Author

vyasr commented Mar 30, 2022

It seems the style check is failing related to black formatting. Are we still missing an update somewhere?

The Docker images haven't been updated yet from the ones we had last night, which were the out-of-date ones. @ajschmidt8 triggered new builds but they don't appear to have been uploaded yet.

@jakirkham
Copy link
Member

rerun tests

1 similar comment
@jakirkham
Copy link
Member

rerun tests

@jakirkham
Copy link
Member

So CI is pulling in the image that was built last night, but we are still seeing a failure here

@jakirkham
Copy link
Member

rerun tests

@jakirkham
Copy link
Member

Hooray the style check passes! 🎉

v22.06 Release automation moved this from PR-WIP to PR-Reviewer approved Apr 1, 2022
@charlesbluca
Copy link
Member

Is there anything else that needs to be done here or are we safe to merge?

@jakirkham
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 921d286 into rapidsai:branch-22.06 Apr 1, 2022
v22.06 Release automation moved this from PR-Reviewer approved to Done Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for review Ready for review by team conda gpuCI improvement Improvement / enhancement to an existing function 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

5 participants