MehulBatra commented on code in PR #8408:
URL: https://github.com/apache/iceberg/pull/8408#discussion_r1306709589
##########
python/pyproject.toml:
##########
@@ -288,3 +281,47 @@ ignore_missing_imports = true
[tool.coverage.run]
source = ['pyiceberg/']
+
+[tool.ruff]
+src = ['pyiceberg/', 'tests/']
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
+ignore = ["E501","E203","B024","B028"]
Review Comment:
added the ignore as well as they were being covered in flake8, post removal
of flake8 added it here.
"W503" is not supported by Ruff so nothing to worry around that.
##########
python/.pre-commit-config.yaml:
##########
@@ -95,3 +69,9 @@ repos:
]
additional_dependencies:
- tomli==2.0.1
+ - repo: https://github.com/astral-sh/ruff-pre-commit
Review Comment:
removed and moved ruff to the top
##########
python/pyproject.toml:
##########
@@ -288,3 +281,47 @@ ignore_missing_imports = true
[tool.coverage.run]
source = ['pyiceberg/']
+
+[tool.ruff]
+src = ['pyiceberg/','/tests']
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
+ignore = ["E501"]
+
+# Allow autofix for all enabled rules (when `--fix`) is provided.
+fixable = ["ALL"]
+unfixable = []
+
+# Exclude a variety of commonly ignored directories.
+exclude = [
+ ".bzr",
+ ".direnv",
+ ".eggs",
+ ".git",
+ ".git-rewrite",
+ ".hg",
+ ".mypy_cache",
+ ".nox",
+ ".pants.d",
+ ".pytype",
+ ".ruff_cache",
+ ".svn",
+ ".tox",
+ ".venv",
+ "__pypackages__",
+ "_build",
+ "buck-out",
+ "build",
+ "dist",
+ "node_modules",
Review Comment:
They come by default when we use Ruff, thought of mentioning them explicitly
if someone wants to go through them, we can delete them if not wanted, they
will still applied behind the scenes
##########
python/pyproject.toml:
##########
@@ -288,3 +281,47 @@ ignore_missing_imports = true
[tool.coverage.run]
source = ['pyiceberg/']
+
+[tool.ruff]
+src = ['pyiceberg/', 'tests/']
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
+ignore = ["E501","E203","B024","B028"]
+
+# Allow autofix for all enabled rules (when `--fix`) is provided.
+fixable = ["ALL"]
+unfixable = []
+
+# Exclude a variety of commonly ignored directories.
+exclude = [
+ ".bzr",
+ ".direnv",
+ ".eggs",
+ ".git",
+ ".git-rewrite",
+ ".hg",
+ ".mypy_cache",
+ ".nox",
+ ".pants.d",
+ ".pytype",
+ ".ruff_cache",
+ ".svn",
+ ".tox",
+ ".venv",
+ "__pypackages__",
+ "_build",
+ "buck-out",
+ "build",
+ "dist",
+ "node_modules",
+ "venv",
+]
+per-file-ignores = {}
+# Ignore _all_ violations.
+# Same as Black.
+line-length = 130
+
+# Allow unused variables when underscore-prefixed.
+dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$"
Review Comment:
on it, will remove the pyUpgrade from pre-commit-config aswell
##########
python/pyiceberg/avro/file.py:
##########
@@ -21,11 +21,11 @@
import io
import json
import os
+from collections.abc import Callable
Review Comment:
Agreed, not sure at what step it started happening, need to check
##########
python/pyproject.toml:
##########
@@ -288,3 +281,47 @@ ignore_missing_imports = true
[tool.coverage.run]
source = ['pyiceberg/']
+
+[tool.ruff]
+src = ['pyiceberg/','/tests']
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
+ignore = ["E501"]
+
+# Allow autofix for all enabled rules (when `--fix`) is provided.
+fixable = ["ALL"]
+unfixable = []
+
+# Exclude a variety of commonly ignored directories.
+exclude = [
+ ".bzr",
+ ".direnv",
+ ".eggs",
+ ".git",
+ ".git-rewrite",
+ ".hg",
+ ".mypy_cache",
+ ".nox",
+ ".pants.d",
+ ".pytype",
+ ".ruff_cache",
+ ".svn",
+ ".tox",
+ ".venv",
+ "__pypackages__",
+ "_build",
+ "buck-out",
+ "build",
+ "dist",
+ "node_modules",
Review Comment:
https://beta.ruff.rs/docs/settings/#exclude
##########
python/.pre-commit-config.yaml:
##########
@@ -95,3 +69,9 @@ repos:
]
additional_dependencies:
- tomli==2.0.1
+ - repo: https://github.com/astral-sh/ruff-pre-commit
Review Comment:
on it!
##########
python/pyproject.toml:
##########
@@ -288,3 +281,47 @@ ignore_missing_imports = true
[tool.coverage.run]
source = ['pyiceberg/']
+
+[tool.ruff]
+src = ['pyiceberg/', 'tests/']
+# Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default.
+# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or
+# McCabe complexity (`C901`) by default.
+select = ["E", "F"]
Review Comment:
Sure, it looks great we can enable these rules and later on once we get
#7776 we can enable 'D' aswell
##########
python/pyiceberg/avro/file.py:
##########
@@ -21,11 +21,11 @@
import io
import json
import os
+from collections.abc import Callable
Review Comment:
fixed, by putting
https://beta.ruff.rs/docs/settings/#isort-detect-same-package
##########
python/.pre-commit-config.yaml:
##########
@@ -28,52 +28,28 @@ repos:
- id: debug-statements
- id: check-yaml
- id: check-ast
+ - repo: https://github.com/astral-sh/ruff-pre-commit
+ # Ruff version (Used for linting)
+ rev: v0.0.286
+ hooks:
+ - id: ruff
+ args: [ --fix, --exit-non-zero-on-fix ]
- repo: https://github.com/ambv/black
rev: 23.3.0
hooks:
- id: black
- - repo: https://github.com/pre-commit/mirrors-isort
- rev: v5.10.1
- hooks:
- - id: isort
- args: [--settings-path=python/pyproject.toml]
+ args: [--skip-string-normalization]
Review Comment:
We are using black and double-quote-strings-fixer together
The former likes double-quoted strings in Python (you can disable this by
configuring black to skip-string-normalization in pyproject.toml)
The latter like single quoted strings in Python (you can remove it if you'd
like double-quoted strings)
If two formatters fight, the end result will be a failure of the CICD so
hence added this.
##########
python/build-module.py:
##########
@@ -17,6 +17,7 @@
import os
import shutil
+
Review Comment:
It separated import and from statements mostly due to property
lines-between-types = 1
##########
python/tests/catalog/test_glue.py:
##########
@@ -19,7 +19,6 @@
import pytest
from moto import mock_glue
-
Review Comment:
let me take care of that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
