From a9919ff9bd3dde3083b650f7bcb294cf43a88734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Thu, 9 Feb 2023 15:13:55 -0600 Subject: [PATCH] ci: Use Ruff to lint project - Auto-fixed unused imports (F401) - Fixed some unused variables and redefined test functions - Fixed docstring conventions in `singer_sdk/helpers/_state.py` - Fixed some imports and removed `isort` from pre-commit Can't get rid of flake8 entirely yet due to https://github.com/charliermarsh/ruff/issues/2459 --- .flake8 | 19 ++-------- .pre-commit-config.yaml | 13 +++---- noxfile.py | 1 - pyproject.toml | 55 +++++++++++++++++++++++----- singer_sdk/helpers/_flattening.py | 4 +-- singer_sdk/helpers/_state.py | 56 +++++++++++------------------ singer_sdk/typing.py | 3 +- tests/core/test_batch.py | 1 - tests/core/test_connector_sql.py | 14 ++++---- tests/core/test_mapper.py | 1 - tests/core/test_metrics.py | 1 - tests/core/test_plugin_config.py | 2 +- tests/core/test_schema.py | 8 +++-- tests/core/test_typing.py | 8 ++--- tests/external/conftest.py | 1 - tests/external/test_tap_gitlab.py | 1 - tests/samples/test_tap_sqlite.py | 21 ++--------- tests/samples/test_target_csv.py | 2 +- tests/samples/test_target_sqlite.py | 7 ++-- 19 files changed, 102 insertions(+), 116 deletions(-) diff --git a/.flake8 b/.flake8 index 12d3b2f342..e9d82b4205 100644 --- a/.flake8 +++ b/.flake8 @@ -1,22 +1,7 @@ [flake8] -ignore = W503, C901, ANN101, ANN102 max-line-length = 88 exclude = cookiecutter per-file-ignores = - # Don't require docstrings or type annotations in tests - # tests/*:D100,D102,D103,DAR,ANN - # Don't require docstrings conventions or type annotations in SDK samples - # samples/*:ANN,DAR - # Don't require docstrings conventions or type annotations in private modules - singer_sdk/helpers/_*.py:ANN,DAR,D105 - # Don't require docstrings conventions in "meta" code - # singer_sdk/helpers/_classproperty.py:D105 - # Ignore unused imports in __init__.py files - singer_sdk/_singerlib/__init__.py:F401 - # Templates support a generic resource of type Any. - singer_sdk/testing/templates.py:ANN401 - # Disabled some checks in samples code - samples/*:ANN,D,DAR -max-complexity = 10 + # Don't require docstrings conventions in private modules + singer_sdk/helpers/_*.py:DAR docstring-convention = google -allow-star-arg-any = true diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9701bf9cc7..4003c1b357 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,6 +35,11 @@ repos: tests/core/test_simpleeval.py )$ +- repo: https://github.com/charliermarsh/ruff-pre-commit + rev: v0.0.253 + hooks: + - id: ruff + - repo: https://github.com/psf/black rev: 23.1.0 hooks: @@ -46,20 +51,12 @@ repos: tests/core/test_simpleeval.py )$ -- repo: https://github.com/pycqa/isort - rev: 5.12.0 - hooks: - - id: isort - exclude: (cookiecutter/.*|singer_sdk/helpers/_simpleeval/.*) - - repo: https://github.com/pycqa/flake8 rev: 6.0.0 hooks: - id: flake8 additional_dependencies: - darglint==1.8.1 - - flake8-annotations==2.9.0 - - flake8-docstrings==1.6.0 files: | (?x)^( singer_sdk/.*| diff --git a/noxfile.py b/noxfile.py index 42e30600f8..84f5f92c51 100644 --- a/noxfile.py +++ b/noxfile.py @@ -73,7 +73,6 @@ def mypy(session: Session) -> None: @session(python=python_versions) def tests(session: Session) -> None: """Execute pytest tests and compute coverage.""" - session.install(".[s3]") session.install(*test_dependencies) diff --git a/pyproject.toml b/pyproject.toml index 4093688ecb..32a2a0c6f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -129,15 +129,6 @@ flake8-docstrings = "^1.7.0" [tool.black] exclude = ".*simpleeval.*" -[tool.isort] -add_imports = [ - "from __future__ import annotations", -] -profile = "black" -multi_line_output = 3 # Vertical Hanging Indent -src_paths = "singer_sdk" -known_first_party = ["tests", "samples"] - [tool.pytest.ini_options] addopts = '-vvv --ignore=singer_sdk/helpers/_simpleeval.py -m "not external"' markers = [ @@ -220,3 +211,49 @@ build-backend = "poetry.core.masonry.api" [tool.poetry.scripts] pytest11 = { callable = "singer_sdk:testing.pytest_plugin", extras = ["testing"] } + +[tool.ruff] +exclude = [ + "cookiecutter/*", +] +ignore = [ + "ANN101", + "ANN102", +] +line-length = 88 +select = [ + "E", + "F", + "ANN", # flake8-annotations + "D", # pydocstyle/flake8-docstrings + "I", # isort +] +src = ["samples", "singer_sdk", "tests"] +target-version = "py37" + +[tool.ruff.per-file-ignores] +"docs/conf.py" = ["D", "I002"] +"noxfile.py" = ["ANN"] +"tests/*" = [ + "ANN", + "D1", + "D2", +] +# Disabled some checks in samples code +"samples/*" = ["ANN", "D"] +# Don't require docstrings conventions or type annotations in private modules +"singer_sdk/helpers/_*.py" = ["ANN", "D105"] +# Templates support a generic resource of type Any. +"singer_sdk/testing/templates.py" = ["ANN401"] + +[tool.ruff.flake8-annotations] +allow-star-arg-any = true +mypy-init-return = true +suppress-dummy-args = true + +[tool.ruff.isort] +known-first-party = ["singer_sdk", "samples", "tests"] +required-imports = ["from __future__ import annotations"] + +[tool.ruff.pydocstyle] +convention = "google" diff --git a/singer_sdk/helpers/_flattening.py b/singer_sdk/helpers/_flattening.py index ed1d275a97..6ab7570b24 100644 --- a/singer_sdk/helpers/_flattening.py +++ b/singer_sdk/helpers/_flattening.py @@ -217,7 +217,7 @@ def _flatten_schema( Args: schema_node: The schema node to flatten. - parent_key: The parent's key, provided as a list of node names. + parent_keys: The parent's key, provided as a list of node names. separator: The string to use when concatenating key names. level: The current recursion level (zero-based). max_level: The max recursion level (zero-based, exclusive). @@ -357,7 +357,7 @@ def _should_jsondump_value(key: str, value: Any, flattened_schema=None) -> bool: Args: key: [description] value: [description] - schema: [description]. Defaults to None. + flattened_schema: [description]. Defaults to None. Returns: [description] diff --git a/singer_sdk/helpers/_state.py b/singer_sdk/helpers/_state.py index 101135462f..bc20eea60f 100644 --- a/singer_sdk/helpers/_state.py +++ b/singer_sdk/helpers/_state.py @@ -22,27 +22,19 @@ def get_state_if_exists( ) -> Any | None: """Return the stream or partition state, creating a new one if it does not exist. - Parameters - ---------- - tap_state : dict - the existing state dict which contains all streams. - tap_stream_id : str - the id of the stream - state_partition_context : Optional[dict], optional - keys which identify the partition context, by default None (not partitioned) - key : Optional[str], optional - name of the key searched for, by default None (return entire state if found) - - Returns - ------- - Optional[Any] + Args: + tap_state: the existing state dict which contains all streams. + tap_stream_id: the id of the stream + state_partition_context: keys which identify the partition context, + by default None (not partitioned) + key: name of the key searched for, by default None (return entire state if + found) + + Returns: Returns the state if exists, otherwise None - Raises - ------ - ValueError - Raised if state is invalid or cannot be parsed. - + Raises: + ValueError: Raised if state is invalid or cannot be parsed. """ if "bookmarks" not in tap_state: return None @@ -106,25 +98,17 @@ def get_writeable_state_dict( ) -> dict: """Return the stream or partition state, creating a new one if it does not exist. - Parameters - ---------- - tap_state : dict - the existing state dict which contains all streams. - tap_stream_id : str - the id of the stream - state_partition_context : Optional[dict], optional - keys which identify the partition context, by default None (not partitioned) - - Returns - ------- - dict - Returns a writeable dict at the stream or partition level. + Args: + tap_state: the existing state dict which contains all streams. + tap_stream_id: the id of the stream + state_partition_context: keys which identify the partition context, + by default None (not partitioned) - Raises - ------ - ValueError - Raise an error if duplicate entries are found. + Returns: + Returns a writeable dict at the stream or partition level. + Raises: + ValueError: Raise an error if duplicate entries are found. """ if tap_state is None: raise ValueError("Cannot write state to missing state dictionary.") diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py index eeb7fab6e8..c4ef9542a1 100644 --- a/singer_sdk/typing.py +++ b/singer_sdk/typing.py @@ -120,7 +120,8 @@ def extend_validator_with_defaults(validator_class): # noqa: ANN001, ANN201 """Fill in defaults, before validating with the provided JSON Schema Validator. - See https://python-jsonschema.readthedocs.io/en/latest/faq/#why-doesn-t-my-schema-s-default-property-set-the-default-on-my-instance # noqa + See + https://python-jsonschema.readthedocs.io/en/latest/faq/#why-doesn-t-my-schema-s-default-property-set-the-default-on-my-instance for details. Args: diff --git a/tests/core/test_batch.py b/tests/core/test_batch.py index 4c991c0215..fef7cab319 100644 --- a/tests/core/test_batch.py +++ b/tests/core/test_batch.py @@ -1,7 +1,6 @@ from __future__ import annotations from dataclasses import asdict -from urllib.parse import urlparse import pytest diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index 76df4cd2e5..a59ba08518 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -55,7 +55,7 @@ def connector(self): "column_name": "old_name", "new_column_name": "new_name", }, - "ALTER TABLE %(table_name)s RENAME COLUMN %(column_name)s to %(new_column_name)s", + "ALTER TABLE %(table_name)s RENAME COLUMN %(column_name)s to %(new_column_name)s", # noqa: E501 "ALTER TABLE full.table.name RENAME COLUMN old_name to new_name", ), ( @@ -70,7 +70,7 @@ def connector(self): "column_name": "column_name", "column_type": sqlalchemy.types.String(), }, - "ALTER TABLE %(table_name)s ALTER COLUMN %(column_name)s (%(column_type)s)", + "ALTER TABLE %(table_name)s ALTER COLUMN %(column_name)s (%(column_type)s)", # noqa: E501 "ALTER TABLE full.table.name ALTER COLUMN column_name (VARCHAR)", ), ], @@ -157,17 +157,17 @@ def test_deprecated_functions_warn(self, connector): def test_connect_calls_engine(self, connector): with mock.patch.object(SQLConnector, "_engine") as mock_engine: - with connector._connect() as conn: + with connector._connect() as _: mock_engine.connect.assert_called_once() - def test_connect_calls_engine(self, connector): + def test_connect_calls_connect(self, connector): attached_engine = connector._engine with mock.patch.object(attached_engine, "connect") as mock_conn: - with connector._connect() as conn: + with connector._connect() as _: mock_conn.assert_called_once() def test_connect_raises_on_operational_failure(self, connector): - with pytest.raises(sqlalchemy.exc.OperationalError) as e: + with pytest.raises(sqlalchemy.exc.OperationalError) as _: with connector._connect() as conn: conn.execute("SELECT * FROM fake_table") @@ -188,6 +188,6 @@ def test_get_slalchemy_url_raises_if_not_in_config(self, connector): def test_dialect_uses_engine(self, connector): attached_engine = connector._engine - with mock.patch.object(attached_engine, "dialect") as mock_dialect: + with mock.patch.object(attached_engine, "dialect") as _: res = connector._dialect assert res == attached_engine.dialect diff --git a/tests/core/test_mapper.py b/tests/core/test_mapper.py index 5b89fd3d63..356b5c854c 100644 --- a/tests/core/test_mapper.py +++ b/tests/core/test_mapper.py @@ -8,7 +8,6 @@ import logging from contextlib import redirect_stdout from pathlib import Path -from typing import Dict, List, Optional import pytest from freezegun import freeze_time diff --git a/tests/core/test_metrics.py b/tests/core/test_metrics.py index 76131847f6..5eb8fa5011 100644 --- a/tests/core/test_metrics.py +++ b/tests/core/test_metrics.py @@ -2,7 +2,6 @@ import logging import time -from textwrap import dedent import pytest diff --git a/tests/core/test_plugin_config.py b/tests/core/test_plugin_config.py index ca4d076fa6..5b26d74428 100644 --- a/tests/core/test_plugin_config.py +++ b/tests/core/test_plugin_config.py @@ -3,7 +3,7 @@ from __future__ import annotations -from typing import Any, Dict, List +from typing import Any from singer_sdk.streams.core import Stream from singer_sdk.tap_base import Tap diff --git a/tests/core/test_schema.py b/tests/core/test_schema.py index 9764f984f6..5fa8c75f87 100644 --- a/tests/core/test_schema.py +++ b/tests/core/test_schema.py @@ -1,7 +1,8 @@ """ Testing that Schema can convert schemas lossless from and to dicts. -Schemas are taken from these examples; https://json-schema.org/learn/miscellaneous-examples.html +Schemas are taken from these examples; +https://json-schema.org/learn/miscellaneous-examples.html NOTE: The following properties are not currently supported; pattern @@ -25,8 +26,9 @@ not Some of these could be trivially added (if they are SIMPLE_PROPERTIES. -Some might need more thinking if they can contain schemas (though, note that we also treat 'additionalProperties', -'anyOf' and' patternProperties' as SIMPLE even though they can contain schemas. +Some might need more thinking if they can contain schemas (though, note that we also +treat 'additionalProperties', 'anyOf' and' patternProperties' as SIMPLE even though they +can contain schemas. """ from __future__ import annotations diff --git a/tests/core/test_typing.py b/tests/core/test_typing.py index 00d9f380fa..b43e7a39ea 100644 --- a/tests/core/test_typing.py +++ b/tests/core/test_typing.py @@ -241,9 +241,9 @@ def test_conform_primitives(): assert _conform_primitive_property(b"\x00", {"type": "string"}) == "00" assert _conform_primitive_property(b"\xBC", {"type": "string"}) == "bc" - assert _conform_primitive_property(b"\x00", {"type": "boolean"}) == False - assert _conform_primitive_property(b"\xBC", {"type": "boolean"}) == True + assert _conform_primitive_property(b"\x00", {"type": "boolean"}) is False + assert _conform_primitive_property(b"\xBC", {"type": "boolean"}) is True assert _conform_primitive_property(None, {"type": "boolean"}) is None - assert _conform_primitive_property(0, {"type": "boolean"}) == False - assert _conform_primitive_property(1, {"type": "boolean"}) == True + assert _conform_primitive_property(0, {"type": "boolean"}) is False + assert _conform_primitive_property(1, {"type": "boolean"}) is True diff --git a/tests/external/conftest.py b/tests/external/conftest.py index ca19a48c08..ef67da9182 100644 --- a/tests/external/conftest.py +++ b/tests/external/conftest.py @@ -4,7 +4,6 @@ import json from pathlib import Path -from typing import Optional import pytest diff --git a/tests/external/test_tap_gitlab.py b/tests/external/test_tap_gitlab.py index 17b0fa2b02..d988208574 100644 --- a/tests/external/test_tap_gitlab.py +++ b/tests/external/test_tap_gitlab.py @@ -1,7 +1,6 @@ from __future__ import annotations import warnings -from typing import Optional from samples.sample_tap_gitlab.gitlab_tap import SampleTapGitlab from singer_sdk._singerlib import Catalog diff --git a/tests/samples/test_tap_sqlite.py b/tests/samples/test_tap_sqlite.py index d64009d31b..1e9216bf49 100644 --- a/tests/samples/test_tap_sqlite.py +++ b/tests/samples/test_tap_sqlite.py @@ -1,32 +1,15 @@ from __future__ import annotations -import json -import sqlite3 -from copy import deepcopy -from io import StringIO from pathlib import Path -from textwrap import dedent -from typing import Dict, cast -from uuid import uuid4 +from typing import cast -import pytest -import sqlalchemy - -from samples.sample_tap_hostile import SampleTapHostile -from samples.sample_tap_sqlite import SQLiteConnector, SQLiteTap from samples.sample_target_csv.csv_target import SampleTargetCSV -from samples.sample_target_sqlite import SQLiteSink, SQLiteTarget from singer_sdk import SQLStream -from singer_sdk import typing as th -from singer_sdk._singerlib import Catalog, MetadataMapping, StreamMetadata +from singer_sdk._singerlib import MetadataMapping, StreamMetadata from singer_sdk.tap_base import SQLTap -from singer_sdk.target_base import SQLTarget from singer_sdk.testing import ( - _get_tap_catalog, get_standard_tap_tests, - tap_sync_test, tap_to_target_sync_test, - target_sync_test, ) diff --git a/tests/samples/test_target_csv.py b/tests/samples/test_target_csv.py index cfbc676196..bdd83ab0ce 100644 --- a/tests/samples/test_target_csv.py +++ b/tests/samples/test_target_csv.py @@ -6,7 +6,7 @@ import shutil import uuid from pathlib import Path -from typing import Any, Dict, List, Optional +from typing import Any import pytest from click.testing import CliRunner diff --git a/tests/samples/test_target_sqlite.py b/tests/samples/test_target_sqlite.py index ce0636f1af..7e60674f19 100644 --- a/tests/samples/test_target_sqlite.py +++ b/tests/samples/test_target_sqlite.py @@ -8,7 +8,6 @@ from io import StringIO from pathlib import Path from textwrap import dedent -from typing import Dict from uuid import uuid4 import pytest @@ -130,7 +129,11 @@ def test_sync_sqlite_to_sqlite( def test_sqlite_schema_addition( sqlite_target_test_config: dict, sqlite_sample_target: SQLTarget ): - """Test that SQL-based targets attempt to create new schema if included in stream name.""" + """Test that SQL-based targets attempt to create new schema. + + It should attempt to create a schema if one is included in stream name, + e.g. "schema_name-table_name". + """ schema_name = f"test_schema_{str(uuid4()).split('-')[-1]}" table_name = f"zzz_tmp_{str(uuid4()).split('-')[-1]}" test_stream_name = f"{schema_name}-{table_name}"