Skip to content

Commit

Permalink
ci: Use Ruff to lint project
Browse files Browse the repository at this point in the history
- 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 astral-sh/ruff#2459
  • Loading branch information
edgarrmondragon committed Feb 27, 2023
1 parent 40a98e9 commit a9919ff
Show file tree
Hide file tree
Showing 19 changed files with 102 additions and 116 deletions.
19 changes: 2 additions & 17 deletions .flake8
Original file line number Diff line number Diff line change
@@ -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
13 changes: 5 additions & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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/.*|
Expand Down
1 change: 0 additions & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
55 changes: 46 additions & 9 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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"
4 changes: 2 additions & 2 deletions singer_sdk/helpers/_flattening.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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]
Expand Down
56 changes: 20 additions & 36 deletions singer_sdk/helpers/_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.")
Expand Down
3 changes: 2 additions & 1 deletion singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion tests/core/test_batch.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

from dataclasses import asdict
from urllib.parse import urlparse

import pytest

Expand Down
14 changes: 7 additions & 7 deletions tests/core/test_connector_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
(
Expand All @@ -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)",
),
],
Expand Down Expand Up @@ -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")

Expand All @@ -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
1 change: 0 additions & 1 deletion tests/core/test_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tests/core/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import logging
import time
from textwrap import dedent

import pytest

Expand Down
2 changes: 1 addition & 1 deletion tests/core/test_plugin_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions tests/core/test_schema.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/core/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion tests/external/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import json
from pathlib import Path
from typing import Optional

import pytest

Expand Down
1 change: 0 additions & 1 deletion tests/external/test_tap_gitlab.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading

0 comments on commit a9919ff

Please sign in to comment.