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

Improve pre-commit hooks and use of ruff #248

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
name: Lint

on: push
on:
push:
branches:
- main
pull_request:
branches:
- main
- planning-1.0-release
workflow_dispatch:

jobs:
ruff:
precommit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: '3.10'
- name: Run Ruff
uses: chartboost/ruff-action@v1
with:
version: 0.5.2

- name: Run pre-commit hooks
run: |
pip install pre-commit
SemyonSinchenko marked this conversation as resolved.
Show resolved Hide resolved
pre-commit install
pre-commit run -a
13 changes: 2 additions & 11 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
repos:
- repo: https://github.com/charliermarsh/ruff-pre-commit
# Ruff version.
rev: 'v0.5.2'
hooks:
- id: ruff
- repo: local
hooks:
- id: pytest
name: pytest-check
entry: poetry run pytest
language: system
pass_filenames: false
# Runs only on python files
types: [ python ]
always_run: true
args: [--exit-non-zero-on-fix]
- id: ruff-format
14 changes: 7 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ install_test: ## Install the 'dev, test and extras' dependencies
install_deps: ## Install all dependencies
@poetry install --with=development,linting,testing,docs

.PHONY: install_ruff
install_ruff: ## Install ruff for use within IDE
@poetry run pip install ruff==0.5.2

.PHONY: update_deps
update_deps: ## Update dependencies
@poetry update --with=development,linting,testing,docs
Expand All @@ -18,13 +22,9 @@ update_deps: ## Update dependencies
test: ## Run all tests
@poetry run pytest tests

.PHONY: lint
lint: ## Lint the code
@poetry run ruff check --fix quinn

.PHONY: format
format: ## Format the code
@poetry run ruff format quinn
.PHONY: check
check: ## Lint and format the code by running pre-commit hooks
@poetry run pre-commit run -a

# Inspired by https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html
.PHONY: help
Expand Down
6 changes: 1 addition & 5 deletions benchmarks/create_benchmark_df.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
def generate_df(spark: SparkSession, n: int) -> DataFrame:
"""Generate a dataframe with a monotonically increasing id column and a random count column."""
count_vals = [(random.randint(1, 10),) for _ in range(n)] # noqa: S311
output: DataFrame = (
spark.createDataFrame(count_vals, schema=["count"])
.withColumn("mvv", F.monotonically_increasing_id())
.select("mvv", "count")
)
output: DataFrame = spark.createDataFrame(count_vals, schema=["count"]).withColumn("mvv", F.monotonically_increasing_id()).select("mvv", "count")
return output


Expand Down
7 changes: 1 addition & 6 deletions benchmarks/visualize_benchmarks.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ def parse_results(spark: SparkSession) -> tuple[pd.DataFrame, pd.DataFrame, str]
["xsmall", "small", "medium", "large"],
)

average_df = (
result_df[["test_name", "dataset_size", "runtime"]]
.groupby(["test_name", "dataset_size"], observed=False)
.mean()
.reset_index()
)
average_df = result_df[["test_name", "dataset_size", "runtime"]].groupby(["test_name", "dataset_size"], observed=False).mean().reset_index()

benchmark_date = get_benchmark_date(benchmark_path="benchmarks/results/")
return result_df, average_df, benchmark_date
Expand Down
130 changes: 102 additions & 28 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ optional = true
[tool.poetry.group.testing]
optional = true

[tool.poetry.group.linting]
optional = true

[tool.poetry.group.development.dependencies]
pyspark = ">3"
semver = "^3"
pre-commit = "*"

[tool.poetry.group.testing.dependencies]
pytest = "^7"
Expand All @@ -61,9 +59,6 @@ pytest-describe = "^2"
pyspark = ">3"
semver = "^3"

[tool.poetry.group.linting.dependencies]
ruff = "0.5.2"

[tool.poetry.group.docs.dependencies]
# All the dependencies related to mkdocs;
# We are pinning only the main version of mkdocs.
Expand All @@ -80,12 +75,15 @@ pymdown-extensions = "*"
mkdocs-macros-plugin = "*"
mkdocs-material-extensions = "*"
markdown-exec = "*"

###########################################################################
# LINTING CONFIGURATION
###########################################################################


[tool.ruff]
line-length = 150
fix = true
SemyonSinchenko marked this conversation as resolved.
Show resolved Hide resolved
extend-exclude = ["tests", "docs"]

[tool.ruff.lint]
Expand Down
1 change: 1 addition & 0 deletions quinn/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# limitations under the License.

"""quinn API."""

from __future__ import annotations

from quinn.append_if_schema_identical import append_if_schema_identical
Expand Down
6 changes: 1 addition & 5 deletions quinn/schema_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,4 @@ def complex_fields(schema: T.StructType) -> dict[str, object]:
:return: A dictionary with complex field names as keys and their respective data types as values.
:rtype: Dict[str, object]
"""
return {
field.name: field.dataType
for field in schema.fields
if isinstance(field.dataType, (T.ArrayType, T.StructType, T.MapType))
}
return {field.name: field.dataType for field in schema.fields if isinstance(field.dataType, (T.ArrayType, T.StructType, T.MapType))}
Loading