Skip to content

Commit

Permalink
Add ruff for Python formatting and linting (streamlit#8849)
Browse files Browse the repository at this point in the history
## Describe your changes

This PR replaces black (formatting), isort (import sorting), and
autoflake (linting) with [ruff](https://github.com/astral-sh/ruff) - an
extremely fast and modern Python linter and code formatter. It also
activates a couple of linting rules that we have been following
implicitly for the last couple of months (e.g., usage of modern
annotations).

## Testing Plan

- No logical changes -> no additional tests required.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
lukasmasuch authored Jun 11, 2024
1 parent a3e99aa commit a36cbcc
Show file tree
Hide file tree
Showing 170 changed files with 626 additions and 663 deletions.
8 changes: 8 additions & 0 deletions .github/codeql-config.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
queries:
# Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
- uses: security-and-quality
query-filters:
- exclude:
id: py/ineffectual-statement
- exclude:
id: py/import-and-import-from
paths:
- lib/streamlit
- frontend/lib/src
Expand Down
18 changes: 10 additions & 8 deletions .github/scripts/build_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@
- The standard output, which means the values will be available in the GitHub logs,
making troubleshooting easier.
"""

from __future__ import annotations

import enum
import fnmatch
import json
import os
import subprocess
import sys
from typing import Dict, List, Optional

if __name__ not in ("__main__", "__mp_main__"):
raise SystemExit(
Expand Down Expand Up @@ -98,7 +100,7 @@ class GithubEvent(enum.Enum):
SCHEDULE = "schedule"


def get_changed_files() -> List[str]:
def get_changed_files() -> list[str]:
"""
Checks the modified files in the last commit.
Expand All @@ -124,14 +126,14 @@ def get_changed_files() -> List[str]:
"--no-commit-id",
"--name-only",
"-r",
f"HEAD^",
"HEAD^",
"HEAD",
]
)
return [line for line in git_output.decode().splitlines() if line]


def get_current_pr_labels() -> List[str]:
def get_current_pr_labels() -> list[str]:
"""
Returns a list of all tags associated with the current PR.
Expand All @@ -146,7 +148,7 @@ def get_current_pr_labels() -> List[str]:
return [label["name"] for label in GITHUB_EVENT["pull_request"].get("labels", [])]


def get_changed_python_dependencies_files() -> List[str]:
def get_changed_python_dependencies_files() -> list[str]:
"""
Gets a list of files that contain Python dependency definitions and have
been modified.
Expand Down Expand Up @@ -176,7 +178,7 @@ def check_if_pr_has_label(label: str, action: str) -> bool:
return False


def get_github_input(input_key: str) -> Optional[str]:
def get_github_input(input_key: str) -> str | None:
"""
Get additional data that the script expects to use during runtime.
Expand Down Expand Up @@ -254,7 +256,7 @@ def is_canary_build() -> bool:
return False


def get_output_variables() -> Dict[str, str]:
def get_output_variables() -> dict[str, str]:
"""
Compute build variables.
"""
Expand Down Expand Up @@ -286,7 +288,7 @@ def get_output_variables() -> Dict[str, str]:
return variables


def save_output_variables(variables: Dict[str, str]) -> None:
def save_output_variables(variables: dict[str, str]) -> None:
"""
Saves build variables
"""
Expand Down
12 changes: 3 additions & 9 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,18 @@ jobs:
language: ["javascript", "python"]

steps:
- name: Checkout repository
- name: Checkout Streamlit code
uses: actions/checkout@v4
with:
persist-credentials: false

submodules: "recursive"
fetch-depth: 2
# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v3
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
# By default, queries listed here will override any specified in a config file.
# Prefix the list here with "+" to use these queries and those in the config file.

# Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
queries: security-and-quality
config-file: ./.github/codeql-config.yml

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v3
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/js-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
run: |
# Run eslint as a standalone command to generate the test report.
# We need to --hook-stage manual to trigger Typecheck too
PRE_COMMIT_NO_CONCURRENCY=true SKIP=eslint pre-commit run --hook-stage manual --show-diff-on-failure --color=always --all-files
PRE_COMMIT_NO_CONCURRENCY=true SKIP=eslint,ruff,ruff-format pre-commit run --hook-stage manual --show-diff-on-failure --color=always --all-files
# Run eslint using Makefile omitting the pre-commit
make jslint
- name: Validate NOTICES
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/python-min-deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ jobs:
python-version: "${{ env.PYTHON_MIN_VERSION }}"
- name: Setup virtual env
uses: ./.github/actions/make_init
with:
use_cached_venv: false
- name: Install min dependencies (force reinstall)
run: uv pip install -r lib/min-constraints-gen.txt --force-reinstall
- name: Generate Protobufs
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/python-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ jobs:
- name: Run make develop
run: make develop
- name: Run Linters
run: |
PRE_COMMIT_NO_CONCURRENCY=true pre-commit run --show-diff-on-failure --color=always --all-files
run: make pylint
env:
RUFF_OUTPUT_FORMAT: github
- name: Run Type Checkers
run: scripts/mypy --report
- name: Run Python Tests
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ eggs/
test-reports
htmlcov
.hypothesis
.ruff_cache

# Test fixtures
cffi_bin
Expand Down
3 changes: 0 additions & 3 deletions .isort.cfg

This file was deleted.

41 changes: 9 additions & 32 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,17 @@
# the second commit should pass,
# because the files were linted after trying to do the first commit.
repos:
- repo: https://github.com/psf/black
rev: 22.6.0
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.4.8
hooks:
- id: black
args:
# Configure Black to support only syntax supported by the minimum supported Python version in setup.py.
- --target-version=py37
# Run the linter.
- id: ruff
args: [--fix]
files: \.py$|\.pyi$
# Run the formatter.
- id: ruff-format
files: \.py$|\.pyi$
- repo: https://github.com/PyCQA/isort
rev: 5.11.5
hooks:
- id: isort
args:
- --filter-files
exclude: ^lib/streamlit/__init__\.py$
- repo: https://github.com/PyCQA/autoflake
rev: v1.7.7
hooks:
- id: autoflake
args:
- "--in-place"
- "--ignore-init-module-imports"
- "--ignore-pass-after-docstring"
- "--remove-unused-variables"
additional_dependencies:
- pyflakes==3.0.1
- repo: local
hooks:
# Script ./scripts/run_in_subdirectory.py was used to work around a
Expand Down Expand Up @@ -108,13 +93,6 @@ repos:
language: system
always_run: true
pass_filenames: false
- id: no-relative-imports
language: pygrep
name: No relative imports
description: Streamlit style is to use absolute imports only (except docs building)
entry: "^\\s*from\\s+\\."
pass_filenames: true
files: \.py$
- repo: https://github.com/Lucas-C/pre-commit-hooks
rev: v1.5.4
hooks:
Expand Down Expand Up @@ -177,7 +155,6 @@ repos:
/vendor/
|^vendor/
|^component-lib/declarations/apache-arrow
|^lib/tests/isolated_asyncio_test_case\.py$
- id: insert-license
name: Add license for all HTML files
files: \.html$
Expand Down
66 changes: 66 additions & 0 deletions .ruff.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# In addition to the standard set of exclusions, omit all tests:
extend-exclude = ["lib/streamlit/proto", "lib/streamlit/emojis.py"]
include = ["lib/**/*.py", "lib/**/*.pyi", "e2e_playwright/**/*.py", "scripts/**/*.py"]
target-version = 'py38'
line-length = 88

[lint]
select = [
"B", # flake8-bugbear
"C4", # Helps you write better list/set/dict comprehensions.
"E", # pycodestyle errors
"FA", # Verifies files use from __future__ import annotations if a type is used in the module that can be rewritten using PEP 563.
"F", # pyflakes
"I", # isort - Import sorting
"ISC", # Encourage correct string literal concatenation.
"LOG", # Checks for issues using the standard library logging module.
"NPY", # Linting rules for numpy
"Q", # Linting rules for quites
"RUF100", # Unused noqa directive
"T20", # Check for Print statements in python files.
"TID", # Helps you write tidier imports.
"UP", # pyupgrade
"W", # pycodestyle warnings
]
ignore = [
"B008", # Checks for function calls in default function arguments.
"B904", # Checks for raise statements in exception handlers that lack a from clause.
"E501", # Checks for lines that exceed the specified maximum character length.
"ISC001", # Checks for implicitly concatenated strings on a single line.
"NPY002", # Checks for the use of legacy np.random function calls.
"PYI036", # Checks for incorrect function signatures on __exit__ and __aexit__ methods.
"PYI041", # Checks for parameter annotations that contain redundant unions between builtin numeric types (e.g., int | float).
"PYI051", # Checks for redundant unions between a Literal and a builtin supertype of that Literal.
"UP031", # Checks for printf-style string formatting, and offers to replace it with str.format calls.
]
# Do not lint files in tests, scripts, and vendor directory:
exclude = ["lib/tests/**", "lib/streamlit/vendor/**", "scripts/**", ".github/**"]

[lint.per-file-ignores]
"e2e_playwright/**" = ["T20", "B018"]
"lib/streamlit/__init__.py" = ["E402"]

[lint.flake8-tidy-imports]
# Disallow all relative imports.
ban-relative-imports = "all"

[lint.isort]
known-first-party = ["streamlit", "shared", "tests", "e2e_playwright"]

[lint.flake8-comprehensions]
# Allow dict calls that make use of keyword arguments (e.g., dict(a=1, b=2)).
allow-dict-calls-with-keyword-arguments = true
23 changes: 12 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -119,22 +119,22 @@ python-init:
fi;\

.PHONY: pylint
# Verify that our Python files are properly formatted.
# Verify that our Python files are properly formatted
# and that there are no lint errors.
pylint:
# Does not modify any files. Returns with a non-zero
# status if anything is not properly formatted. (This isn't really
# "linting"; we're not checking anything but code style.)
if command -v "black" > /dev/null; then \
$(BLACK) --diff --check lib/streamlit/ --exclude=/*_pb2.py$/ && \
$(BLACK) --diff --check lib/tests/ && \
$(BLACK) --diff --check e2e/scripts/ ; \
fi
# Checks if the formatting is correct:
ruff format --check
# Run linter:
ruff check

.PHONY: pyformat
# Fix Python files that are not properly formatted.
# https://docs.astral.sh/ruff/formatter/#sorting-imports
pyformat:
pre-commit run black --all-files --hook-stage manual
pre-commit run isort --all-files --hook-stage manual
# Sort imports:
ruff check --select I --fix
# Run code formatter
ruff format

.PHONY: pytest
# Run Python unit tests.
Expand Down Expand Up @@ -216,6 +216,7 @@ clean:
find . -name __pycache__ -type d -delete || true
find . -name .pytest_cache -exec rm -rfv {} \; || true
rm -rf .mypy_cache
rm -rf .ruff_cache
rm -f lib/streamlit/proto/*_pb2.py*
rm -rf lib/streamlit/static
rm -f lib/Pipfile.lock
Expand Down
2 changes: 1 addition & 1 deletion e2e/scripts/iframe_resizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@

x = st.slider("Enter a number", 0, 20, 0)

for i in range(x):
for _ in range(x):
st.write("Hello example")
1 change: 0 additions & 1 deletion e2e/scripts/redisplayed_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from streamlit import runtime

if runtime.exists():

if st.checkbox("checkbox 1"):
if st.checkbox("checkbox 2"):
st.write("hello")
Expand Down
9 changes: 5 additions & 4 deletions e2e_playwright/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
Global pytest fixtures for e2e tests.
This file is automatically run by pytest before tests are executed.
"""

from __future__ import annotations

import hashlib
Expand All @@ -33,7 +34,7 @@
from random import randint
from tempfile import TemporaryFile
from types import ModuleType
from typing import Any, Callable, Dict, Generator, List, Literal, Protocol, Tuple
from typing import Any, Callable, Generator, Literal, Protocol
from urllib import parse

import pytest
Expand Down Expand Up @@ -257,7 +258,7 @@ def app(page: Page, app_port: int) -> Page:
@pytest.fixture(scope="function")
def app_with_query_params(
page: Page, app_port: int, request: FixtureRequest
) -> Tuple[Page, Dict]:
) -> tuple[Page, dict]:
"""Fixture that opens the app with additional query parameters.
The query parameters are passed as a dictionary in the 'param' key of the request.
"""
Expand Down Expand Up @@ -362,7 +363,7 @@ def _expect_streamlit_app_loaded_in_iframe_with_added_header(


@pytest.fixture(scope="session")
def browser_type_launch_args(browser_type_launch_args: Dict, browser_name: str):
def browser_type_launch_args(browser_type_launch_args: dict, browser_name: str):
"""Fixture that adds the fake device and ui args to the browser type launch args."""
# The browser context fixture in pytest-playwright is defined in session scope, and
# depends on the browser_type_launch_args fixture. This means that we can't
Expand Down Expand Up @@ -479,7 +480,7 @@ def assert_snapshot(

snapshot_default_file_name: str = test_function_name + snapshot_file_suffix

test_failure_messages: List[str] = []
test_failure_messages: list[str] = []

def compare(
element: ElementHandle | Locator | Page,
Expand Down
Loading

0 comments on commit a36cbcc

Please sign in to comment.