-
Notifications
You must be signed in to change notification settings - Fork 60
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] Investigate using black #23
Comments
I think the two issues are: |
Yeah the matrix point makes sense. Disabling formatting on them seems fine On 2 it might be worth asking scikit-image if they are willing to use black. We started using this a while back in Dask + Distributed as well as RAPIDS, which has helped keep things readable and consistent |
It did come up in the past in scikit-image/scikit-image#4111 and wasn't received too favorably at the time. Maybe people are more familiar with it by now, though? I am not too strongly against using it here even if upstream does not. We can always apply black to the scikit-image source prior to doing a diff if we need to compare files directly across the two. For specific files like |
Thanks for the context. Yeah I think black has also softened on some things, which helps, and more projects have started to use it, which also helps issue discovery & subsequent fixes That said, if it is going to make it hard to coordinates changes between here and scikit-image. It's probably not worth the effort until scikit-image adopts it |
…ect.toml (#616) The first commit here is from #615 This MR makes multiple linting-related changes to cuCIM - moves existing `isort` config from `setup.cfg` to a new `pyproject.toml` file - update `isort` conventions to match other RAPIDS projects - add `black` and apply it (use `fmt::off`/`fmt::on` as needed to preserve formatting of look-up tables and test cases) - replace use of `flake8` with `ruff` - add config for `codespell` and apply recommended fixes across both C++ and Python code - update CONTRIBUTING.md instructions to reflect these changes - Pre-commit hooks for `black`, `ruff` and `codespell` were added. The pre-commit hook for `flake8` was removed. The only non-linting change is - move pytest configuration from `setup.cfg` to `pyproject.toml` I plan to leave any build-system related changes to `pyproject.toml` to a follow-up MR. This MR focuses only on linting/testing. It will probably be easiest to review the individual commits. Some, like applying `black` are quite large. Fixes #23 Authors: - Gregory Lee (https://github.com/grlee77) Approvers: - Bradley Dice (https://github.com/bdice) - Ray Douglass (https://github.com/raydouglass) - Gigon Bae (https://github.com/gigony) - https://github.com/jakirkham URL: #616
Previously we tried to use black for formatting Python, but ran into some issues. It would be good to revisit this at some point
cc @grlee77 @quasiben
The text was updated successfully, but these errors were encountered: