Skip to content

Commit

Permalink
Replace black and flake8 with ruff (#75)
Browse files Browse the repository at this point in the history
Co-authored-by: J.C. Jones <[email protected]>
  • Loading branch information
cclauss and jcjones authored Feb 29, 2024
1 parent 5bc30a0 commit 57ef9fe
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 74 deletions.
4 changes: 0 additions & 4 deletions .flake8

This file was deleted.

21 changes: 9 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,16 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Python 3.9
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: 3.9

- name: Install Linting Tools
run: |
python -m pip install --upgrade pip
pip install --user pylint==3.1.0
pip install --user black~=23.9.1
pip install --user flake8~=6.1.0
pip install --user pytest
pip install --user pylint==3.1.0 pytest ruff
- name: Install Partition Manager
run: |
Expand All @@ -30,21 +27,21 @@ jobs:
run: |
python -m pylint -E partitionmanager
- name: Checking format with Black
- name: Checking format with Ruff
run: |
python -m black --check .
python -m ruff format .
- name: Checking format with Flake8
- name: Lint Python code with Ruff
run: |
python -m flake8
python -m ruff --output-format=github
test:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Python 3.9
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: 3.9
- name: Install Partition Manager
Expand Down
17 changes: 9 additions & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
rev: v4.5.0
hooks:
- id: check-ast
- id: check-merge-conflict
- id: detect-private-key
- id: end-of-file-fixer
- id: requirements-txt-fixer
- id: trailing-whitespace
- repo: https://github.com/psf/black
rev: "23.9.1"

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.2.2
hooks:
- id: black
- repo: https://github.com/pycqa/flake8
rev: "6.1.0"
hooks:
- id: flake8
- id: ruff
# - id: ruff-format

- repo: https://github.com/PyCQA/pylint
rev: v3.1.0
hooks:
Expand All @@ -26,6 +25,8 @@ repos:
- PyMySQL
- pyyaml
- pytest
- setuptools

- repo: local
hooks:
- id: pytest
Expand Down
2 changes: 1 addition & 1 deletion partitionmanager/database_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def calculate_exact_timestamp_via_query(database, table, position_partition):
raise partitionmanager.types.NoExactTimeException(
"Unexpected column count for the timestamp result"
)
for key, value in exact_time_result[0].items():
for value in exact_time_result[0].values():
exact_time = datetime.fromtimestamp(value, tz=timezone.utc)
break

Expand Down
11 changes: 6 additions & 5 deletions partitionmanager/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,7 @@ def _generate_sql_copy_commands(
yield f"\tPARTITION {max_val_part.name} VALUES LESS THAN {max_val_string}"
yield ");"

for command in alter_commands_iter:
yield command
yield from alter_commands_iter

cols = set(columns)

Expand Down Expand Up @@ -214,9 +213,11 @@ def _generate_sql_copy_commands(
):
yield line

yield "\t\tWHERE " + " AND ".join(
_trigger_column_copies(map_data["range_cols"])
) + ";"
yield (
"\t\tWHERE "
+ " AND ".join(_trigger_column_copies(map_data["range_cols"]))
+ ";"
)

return

Expand Down
2 changes: 1 addition & 1 deletion partitionmanager/stats_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_statistics_two_partitions(self):
def test_statistics_weekly_partitions_year(self):
parts = list()
base = datetime(2020, 5, 20, tzinfo=timezone.utc)
for w in range(0, 52):
for w in range(52):
partName = f"p_{base + timedelta(weeks=w):%Y%m%d}"
parts.append(mkPPart(partName, w * 1024))
parts.append(MaxValuePartition(f"p_{base + timedelta(weeks=52):%Y%m%d}", 1))
Expand Down
15 changes: 7 additions & 8 deletions partitionmanager/table_append_partition.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ def _parse_partition_map(rows):

options = rows[0]

for l in options["Create Table"].split("\n"):
range_match = partition_range.match(l)
for line in options["Create Table"].split("\n"):
range_match = partition_range.match(line)
if range_match:
range_cols = [x.strip("` ") for x in range_match.group("cols").split(",")]
log.debug(f"Partition range columns: {range_cols}")

member_match = partition_member.match(l)
member_match = partition_member.match(line)
if member_match:
part_name = member_match.group("name")
part_vals_str = member_match.group("cols")
Expand All @@ -139,7 +139,7 @@ def _parse_partition_map(rows):
)
partitions.append(pos_part)

member_tail = partition_tail.match(l)
member_tail = partition_tail.match(line)
if member_tail:
if range_cols is None:
raise partitionmanager.types.TableInformationException(
Expand Down Expand Up @@ -200,7 +200,7 @@ def _split_partitions_around_position(partition_list, current_position):
if not partitionmanager.types.is_partition_type(p):
raise partitionmanager.types.UnexpectedPartitionException(p)
if not isinstance(current_position, partitionmanager.types.Position):
raise ValueError()
raise ValueError

less_than_partitions = list()
greater_or_equal_partitions = list()
Expand Down Expand Up @@ -481,7 +481,7 @@ def _plan_partition_changes(
"as without an empty partition to manipulate, you'll need to "
"perform an expensive copy operation. See the bootstrap mode."
)
raise partitionmanager.types.NoEmptyPartitionsAvailableException()
raise partitionmanager.types.NoEmptyPartitionsAvailableException
if not active_partition:
raise Exception("Active Partition can't be None")

Expand Down Expand Up @@ -626,8 +626,7 @@ def _should_run_changes(table, altered_partitions):
log.debug(f"{p} is new")
return True

if isinstance(p, partitionmanager.types.ChangePlannedPartition):
if p.important():
if isinstance(p, partitionmanager.types.ChangePlannedPartition) and p.important():
log.debug(f"{p} is marked important")
return True
return False
Expand Down
7 changes: 2 additions & 5 deletions partitionmanager/table_append_partition_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# flake8: noqa: E501
# ruff: noqa: E501

import unittest
import argparse
Expand All @@ -7,11 +7,8 @@
ChangePlannedPartition,
DatabaseCommand,
DuplicatePartitionException,
MaxValuePartition,
MismatchedIdException,
NewPlannedPartition,
NoEmptyPartitionsAvailableException,
PositionPartition,
InstantPartition,
SqlInput,
SqlQuery,
Expand Down Expand Up @@ -49,7 +46,7 @@ def __init__(self):
self._response = []
self._num_queries = 0

def run(self, cmd):
def run(self, cmd): # noqa: ARG002
self._num_queries += 1
return self._response.pop()

Expand Down
47 changes: 17 additions & 30 deletions partitionmanager/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ def timedelta_from_dict(r):
for k, v in r.items():
if k == "days":
return timedelta(days=v)
raise argparse.ArgumentTypeError(
f"Unknown retention period definition: {k}={v}"
)
raise argparse.ArgumentTypeError(f"Unknown retention period definition: {k}={v}")
return None


class Table:
Expand Down Expand Up @@ -96,32 +95,22 @@ def __new__(cls, *args):
raise argparse.ArgumentTypeError(f"{args} is not a single argument")
query_string = args[0].strip()
if not query_string.endswith(";"):
raise argparse.ArgumentTypeError(
f"[{query_string}] does not end with a ';'"
)
raise argparse.ArgumentTypeError(f"[{query_string}] does not end with a ';'")
if query_string.count(";") > 1:
raise argparse.ArgumentTypeError(
f"[{query_string}] has more than one statement"
)
raise argparse.ArgumentTypeError(f"[{query_string}] has more than one statement")

if "?" not in query_string:
raise argparse.ArgumentTypeError(
f"[{query_string}] has no substitution variable '?'"
)
raise argparse.ArgumentTypeError(f"[{query_string}] has no substitution variable '?'")
if query_string.count("?") > 1:
raise argparse.ArgumentTypeError(
f"[{query_string}] has more than one substitution variable '?'"
)

if not query_string.upper().startswith("SELECT "):
raise argparse.ArgumentTypeError(
f"[{query_string}] is not a SELECT statement"
)
raise argparse.ArgumentTypeError(f"[{query_string}] is not a SELECT statement")
for term in SqlQuery.forbidden_terms:
if term in query_string.upper():
raise argparse.ArgumentTypeError(
f"[{query_string}] has a forbidden term [{term}]"
)
raise argparse.ArgumentTypeError(f"[{query_string}] has a forbidden term [{term}]")

return super().__new__(cls, query_string)

Expand All @@ -142,7 +131,7 @@ def to_sql_url(urlstring):
urltuple = urlparse(urlstring)
if urltuple.scheme.lower() != "sql":
raise argparse.ArgumentTypeError(f"{urlstring} is not a valid sql://")
if urltuple.path == "/" or urltuple.path == "":
if urltuple.path in {"/", ""}:
raise argparse.ArgumentTypeError(f"{urlstring} should include a db path")
return urltuple
except ValueError as ve:
Expand Down Expand Up @@ -259,7 +248,7 @@ def set_position(self, position_in):
"""Set the list of identifiers for this position."""
if isinstance(position_in, Position):
self._position = position_in.as_list()
elif isinstance(position_in, list) or isinstance(position_in, tuple):
elif isinstance(position_in, (list, tuple)):
self._position = [int(p) for p in position_in]
else:
raise ValueError(f"Unexpected position input: {position_in}")
Expand Down Expand Up @@ -345,10 +334,10 @@ def __lt__(self, other):

# If ALL of v_mine >= v_other, then self is greater than other
# If ANY of v_mine < v_other, then self is less than other
for v_mine, v_other in zip(self._position.as_list(), other_position_list):
if v_mine < v_other:
return True
return False
return any(
v_mine < v_other
for v_mine, v_other in zip(self._position.as_list(), other_position_list)
)

def __ge__(self, other):
return not self < other
Expand Down Expand Up @@ -388,7 +377,7 @@ def values(self):

def __lt__(self, other):
"""MaxValuePartitions are always greater than every other partition."""
if isinstance(other, list) or isinstance(other, Position):
if isinstance(other, (Position, list)):
if self._count != len(other):
raise UnexpectedPartitionException(
f"Expected {self._count} columns but list has {len(other)}."
Expand Down Expand Up @@ -506,11 +495,9 @@ def set_as_max_value(self):
def as_partition(self):
"""Return a concrete Partition that can be rendered into a SQL ALTER."""
if not self._timestamp:
raise ValueError()
raise ValueError
if self._position:
return PositionPartition(f"p_{self._timestamp:%Y%m%d}").set_position(
self._position
)
return PositionPartition(f"p_{self._timestamp:%Y%m%d}").set_position(self._position)
return MaxValuePartition(f"p_{self._timestamp:%Y%m%d}", count=self._num_columns)

def __repr__(self):
Expand All @@ -535,7 +522,7 @@ class ChangePlannedPartition(_PlannedPartition):

def __init__(self, old_part):
if not is_partition_type(old_part):
raise ValueError()
raise ValueError
super().__init__()
self._old = old_part
self._num_columns = self._old.num_columns
Expand Down
Loading

0 comments on commit 57ef9fe

Please sign in to comment.