Skip to content

Commit

Permalink
Replace flake8 with ruff (#1101)
Browse files Browse the repository at this point in the history
## Description of Changes
Replacing `flake8` with `ruff`, which has approximate parity and is more
than 10x faster:
https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-flake8

## Tests and Linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.
  • Loading branch information
michplunkett authored Jun 13, 2024
1 parent 051aa00 commit d11b9a2
Show file tree
Hide file tree
Showing 14 changed files with 138 additions and 164 deletions.
15 changes: 8 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,15 @@ repos:
args:
- --quiet

- repo: https://github.com/PyCQA/flake8
rev: 6.0.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.7
hooks:
- id: flake8
args:
- --ignore=E203,E402,E501,E800,W503,W391,E261,W504
- --select=B,C,E,F,W,T4,B9
- --exclude=./OpenOversight/app/db_repository/versions/
- id: ruff
types_or: [python, pyi, jupyter]
args:
- --fix
- --select=B,C,E,F,W
- --ignore=C901,E203,E402,E501,W391,E261

- repo: https://github.com/psf/black
rev: 23.7.0
Expand Down
54 changes: 22 additions & 32 deletions OpenOversight/app/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,7 @@ def update_officer_field(officer_field_name):
):
ImportLog.log_change(
officer,
"Updated {}: {} --> {}".format(
officer_field_name,
getattr(officer, officer_field_name),
row[officer_field_name],
),
f"Updated {officer_field_name}: {getattr(officer, officer_field_name)} --> {row[officer_field_name]}",
)
setattr(officer, officer_field_name, row[officer_field_name])

Expand Down Expand Up @@ -230,30 +226,23 @@ def update_officer_field(officer_field_name):
new_value = parse(row[field_name])
if isinstance(old_value, date):
new_value = new_value.date()
except Exception as e:
except Exception as err:
msg = (
'Field {} is a date-type, but "{}" was specified for '
"Officer {} {} and cannot be parsed as a date-type.\nError "
"message from dateutil: {}".format(
field_name,
row[field_name],
officer.first_name,
officer.last_name,
e,
)
f'Field {field_name} is a date-type, but "{row[field_name]}"'
f" was specified for Officer {officer.first_name} "
f"{officer.last_name} and cannot be parsed as a "
f"date-type.\nError message from dateutil: {err}"
)
raise Exception(msg)
raise Exception(msg) from err
else:
new_value = row[field_name]
if old_value is None:
update_officer_field(field_name)
elif str(old_value) != str(new_value):
msg = "Officer {} {} has differing {} field. Old: {}, new: {}".format(
officer.first_name,
officer.last_name,
field_name,
old_value,
new_value,
msg = (
f"Officer {officer.first_name} {officer.last_name} has "
f"differing {field_name} field. Old: {old_value}, "
f"new: {new_value}"
)
if update_static_fields:
print(msg)
Expand Down Expand Up @@ -338,23 +327,25 @@ def process_assignment(row, officer, compare=False):
.all()
)
for assignment, job in assignments:
assignment_fieldnames = [
assignment_field_names = [
"star_no",
"unit_id",
"start_date",
"resign_date",
]
i = 0
for fieldname in assignment_fieldnames:
current = getattr(assignment, fieldname)
for field_name in assignment_field_names:
current = getattr(assignment, field_name)
# Test if fields match between row and existing assignment
if (
current
and fieldname in row
and is_equal(row[fieldname], current)
) or (not current and (fieldname not in row or not row[fieldname])):
and field_name in row
and is_equal(row[field_name], current)
) or (
not current and (field_name not in row or not row[field_name])
):
i += 1
if i == len(assignment_fieldnames):
if i == len(assignment_field_names):
job_title = job.job_title
if (
job_title and row.get("job_title", "Not Sure") == job_title
Expand Down Expand Up @@ -537,9 +528,8 @@ def bulk_add_officers(filename, no_create, update_by_name, update_static_fields)
)
else:
raise Exception(
"Officer {} {} missing badge number and unique identifier".format(
row["first_name"], row["last_name"]
)
f"Officer {row['first_name']} {row['last_name']} "
"missing badge number and unique identifier"
)
else:
officer = Officer.query.filter_by(
Expand Down
26 changes: 9 additions & 17 deletions OpenOversight/app/csv_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,21 +208,17 @@ def _handle_assignments_csv(
officer_id = row["officer_id"]
if officer_id != "" and officer_id[0] != "#":
all_rel_officers.add(int(officer_id))
wrong_department = all_rel_officers - set(
[int(oid) for oid in all_officers.keys() if oid[0] != "#"]
)
wrong_department = all_rel_officers - {
int(oid) for oid in all_officers.keys() if oid[0] != "#"
}
if len(wrong_department) > 0:
raise Exception(
"Referenced {} officers in assignment csv that belong to different "
"department. Example ids: {}".format(
len(wrong_department),
", ".join(map(str, list(wrong_department)[:3])),
)
f"Referenced {len(wrong_department)} officers in assignment "
"csv that belong to different department. Example IDs"
f": {', '.join(map(str, list(wrong_department)[:3]))}"
)
print(
"Deleting assignments from {} officers to overwrite.".format(
len(all_rel_officers)
)
f"Deleting assignments from {len(all_rel_officers)} officers to overwrite."
)
(
db.session.query(Assignment)
Expand All @@ -245,9 +241,7 @@ def _handle_assignments_csv(
officer = all_officers.get(row["officer_id"])
if not officer:
raise Exception(
"Officer with id {} does not exist (in this department)".format(
row["officer_id"]
)
f"Officer with id {row['officer_id']} does not exist (in this department)"
)
if row.get("unit_id"):
assert (
Expand Down Expand Up @@ -331,9 +325,7 @@ def _handle_salaries(
officer = all_officers.get(row["officer_id"])
if not officer:
raise Exception(
"Officer with id {} does not exist (in this department)".format(
row["officer_id"]
)
f"Officer with id {row['officer_id']} does not exist (in this department)"
)
row["officer_id"] = officer.id
_create_or_update_model(
Expand Down
10 changes: 5 additions & 5 deletions OpenOversight/app/formfields.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def _value(self):
else:
return self.data and self.data.strftime(self.format) or ""

def process_formdata(self, valuelist):
if valuelist and valuelist != [""]:
time_str = " ".join(valuelist)
def process_formdata(self, values):
if values and values != [""]:
time_str = " ".join(values)
try:
components = time_str.split(":")
hour = 0
Expand All @@ -36,6 +36,6 @@ def process_formdata(self, valuelist):
else:
raise ValueError
self.data = datetime.time(hour, minutes, seconds)
except ValueError:
except ValueError as err:
self.data = None
raise ValueError(self.gettext("Not a valid time"))
raise ValueError(self.gettext("Not a valid time")) from err
2 changes: 1 addition & 1 deletion OpenOversight/app/main/model_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def populate_obj(self, form, obj):
form.populate_obj(obj)

# if the object doesn't have a creator id set it to current user
if hasattr(obj, "created_by") and not getattr(obj, "created_by"):
if hasattr(obj, "created_by") and not obj.created_by:
obj.created_by = current_user.id
# if the object keeps track of who updated it last, set to current user
if hasattr(obj, "last_updated_by"):
Expand Down
6 changes: 3 additions & 3 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def edit_assignment(officer_id: int, assignment_id: int):
)
form.job_title.data = Job.query.filter_by(id=assignment.job_id).one()
form.unit.query = unit_choices(officer.department_id)
if form.unit.data and type(form.unit.data) == int:
if form.unit.data and isinstance(form.unit.data, int):
form.unit.data = Unit.query.filter_by(id=form.unit.data).one()
if form.validate_on_submit():
form.job_title.data = Job.query.filter_by(
Expand Down Expand Up @@ -1087,7 +1087,7 @@ def get_dept_ranks(department_id: Optional[int] = None, is_sworn_officer: bool =
ranks = Job.query.all()
# Prevent duplicate ranks
rank_list = sorted(
set((rank.id, rank.job_title) for rank in ranks),
{(rank.id, rank.job_title) for rank in ranks},
key=lambda x: x[1],
)

Expand Down Expand Up @@ -1117,7 +1117,7 @@ def get_dept_units(department_id: Optional[int] = None):
units = Unit.query.all()
# Prevent duplicate units
unit_list = sorted(
set((unit.id, unit.description) for unit in units),
{(unit.id, unit.description) for unit in units},
key=lambda x: x[1],
)

Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/utils/cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ def save_image_to_s3_and_db(image_buf, user_id, department_id=None):
image_buf.seek(0)
try:
pimage = Pimage.open(image_buf)
except UnidentifiedImageError:
raise ValueError("Attempted to pass an invalid image.")
except UnidentifiedImageError as err:
raise ValueError("Attempted to pass an invalid image.") from err
image_format = pimage.format.lower()
if image_format not in current_app.config[KEY_ALLOWED_EXTENSIONS]:
raise ValueError(f"Attempted to pass invalid data type: {image_format}")
Expand Down
118 changes: 59 additions & 59 deletions OpenOversight/app/utils/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,46 +86,46 @@ def add_officer_profile(form: AddOfficerForm, user: User) -> Officer:
)
db.session.add(assignment)
if form.links.data:
for link in form.data["links"]:
if link["url"]:
li = get_or_create_link_from_form(link, user)
officer.links.append(li)
form_links = [link for link in form.data["links"] if link["url"]]
for link in form_links:
li = get_or_create_link_from_form(link, user)
officer.links.append(li)
if form.notes.data:
for note in form.data["notes"]:
# don't try to create with a blank string
if note["text_contents"]:
new_note = Note(
text_contents=note["text_contents"],
officer=officer,
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_note)
# don't try to create with a blank string
form_notes = [n for n in form.data["notes"] if n["text_contents"]]
for note in form_notes:
new_note = Note(
text_contents=note["text_contents"],
officer=officer,
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_note)
if form.descriptions.data:
for description in form.data["descriptions"]:
# don't try to create with a blank string
if description["text_contents"]:
new_description = Description(
text_contents=description["text_contents"],
officer=officer,
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_description)
# don't try to create with a blank string
form_descriptions = [d for d in form.data["descriptions"] if d["text_contents"]]
for description in form_descriptions:
new_description = Description(
text_contents=description["text_contents"],
officer=officer,
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_description)
if form.salaries.data:
for salary in form.data["salaries"]:
# don't try to create with a blank string
if salary["salary"]:
new_salary = Salary(
officer=officer,
salary=salary["salary"],
overtime_pay=salary["overtime_pay"],
year=salary["year"],
is_fiscal_year=salary["is_fiscal_year"],
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_salary)
# don't try to create with a blank string
form_salaries = [s for s in form.data["salaries"] if s["salary"]]
for salary in form_salaries:
new_salary = Salary(
officer=officer,
salary=salary["salary"],
overtime_pay=salary["overtime_pay"],
year=salary["year"],
is_fiscal_year=salary["is_fiscal_year"],
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(new_salary)

db.session.commit()
return officer
Expand Down Expand Up @@ -171,34 +171,34 @@ def create_incident(self, form: IncidentForm, user: User) -> Incident:
address_model = location

if "officers" in form.data:
for officer in form.data["officers"]:
if officer["oo_id"]:
of = Officer.query.filter_by(id=int(officer["oo_id"])).one()
if of:
officers.append(of)
form_officers = [o for o in form.data["officers"] if o["oo_id"]]
for officer in form_officers:
of = Officer.query.filter_by(id=int(officer["oo_id"])).one()
if of:
officers.append(of)

if "license_plates" in form.data:
for plate in form.data["license_plates"]:
if plate["number"]:
lp = LicensePlate.query.filter_by(
form_plates = [p for p in form.data["license_plates"] if p["number"]]
for plate in form_plates:
lp = LicensePlate.query.filter_by(
number=if_exists_or_none(plate["number"]),
state=if_exists_or_none(plate["state"]),
).first()
if not lp:
lp = LicensePlate(
number=if_exists_or_none(plate["number"]),
state=if_exists_or_none(plate["state"]),
).first()
if not lp:
lp = LicensePlate(
number=if_exists_or_none(plate["number"]),
state=if_exists_or_none(plate["state"]),
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(lp)
license_plates.append(lp)
created_by=user.id,
last_updated_by=user.id,
)
db.session.add(lp)
license_plates.append(lp)

if "links" in form.data:
for link in form.data["links"]:
if link["url"]:
li = get_or_create_link_from_form(link, user)
links.append(li)
form_links = [link for link in form.data["links"] if link["url"]]
for link in form_links:
li = get_or_create_link_from_form(link, user)
links.append(li)

return Incident(
address=address_model,
Expand Down
2 changes: 1 addition & 1 deletion OpenOversight/app/utils/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def get_or_create(session, model, defaults=None, **kwargs):
if instance:
return instance, False
else:
params = dict((k, v) for k, v in filter_params.items())
params = dict(filter_params)
params.update(defaults or {})
instance = model(**params)
session.add(instance)
Expand Down
Loading

0 comments on commit d11b9a2

Please sign in to comment.