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

style: replace black+isort+flake8 tooling with ruff #1466

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

nfarabullini
Copy link
Contributor

@nfarabullini nfarabullini commented Feb 22, 2024

Code style and format changes moving from black+isort+flake8 to ruff.

@nfarabullini nfarabullini marked this pull request as draft February 22, 2024 13:20
@nfarabullini nfarabullini marked this pull request as ready for review February 23, 2024 12:21
@nfarabullini nfarabullini changed the title ruff implementation refactor[all]: ruff implementation Feb 23, 2024
@egparedes egparedes changed the title refactor[all]: ruff implementation style: replace black+isort+flake8 tooling by ruff Feb 26, 2024
@egparedes egparedes changed the title style: replace black+isort+flake8 tooling by ruff style: replace black+isort+flake8 tooling with ruff Feb 26, 2024
Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the first round of feedback. Didn't have time to review all the changes yet but I'll do in the second pass, once this comments and the merge conflicts are fixed.

.pre-commit-config.yaml Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
src/gt4py/next/ffront/field_operator_ast.py Outdated Show resolved Hide resolved
src/gt4py/next/ffront/field_operator_ast.py Outdated Show resolved Hide resolved
src/gt4py/next/ffront/field_operator_ast.py Show resolved Hide resolved
src/gt4py/next/ffront/field_operator_ast.py Outdated Show resolved Hide resolved
src/gt4py/next/ffront/field_operator_ast.py Show resolved Hide resolved
Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great. Thanks for all the work!

@egparedes egparedes merged commit 6a19355 into GridTools:main Feb 28, 2024
55 checks passed
@havogt havogt mentioned this pull request Feb 29, 2024
4 tasks
egparedes pushed a commit that referenced this pull request Sep 3, 2024
In PR #1466, `flake8`, `black`,
and `isort` were replaced with the [ruff](https://astral.sh/ruff) linter
and formatter. It seems there are a few leftovers of this transition. In
this PR, I did a quick search for flake8 and deleted / replaced all
instances found.

Removing `flake8` means rebuilding the requirements (to propagate the
changes to the frozen `requirements-*` files), which updates a couple of
dependencies.

We also decided to remove the `linter-*` tasks of `tox` since they have
been replaced by `pre-commit`.
romanc pushed a commit to romanc/gt4py that referenced this pull request Sep 6, 2024
isort has been replaced by ruff with PR
GridTools#1466.

Removing isort means rebuilding the requirements, which updates a couple
of unrelated (to the isort removal) dependencies.
egparedes pushed a commit that referenced this pull request Sep 11, 2024
`isort` has been replaced by `ruff` in PR
#1466.

_Note_ Removing `isort` means rebuilding the requirements, which updates
a couple of unrelated (to the `isort` removal) dependencies.

_Note 2_ I also checked for `black` (formatter) and there are lingering
references too. However, `black` is actively used as formatter in code
generation


https://github.com/GridTools/gt4py/blob/70dd7c34036e48c37f5047bf32573916836a4a35/src/gt4py/eve/codegen.py#L106

Co-authored-by: Roman Cattaneo <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants