Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable many more Ruff linter rules #9283

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
6 changes: 6 additions & 0 deletions .cookiecutter/includes/ruff/lint/per_file_ignores/tests/tail
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"PT006", # Enforces a consistent style for the type of the `argnames` parameter to
# pytest.mark.parametrize. We have too many pre-existing violations of
# this.
"PT007", # Enforces a consistent style for the type of the `argvalues` parameter to
# pytest.mark.parametrize. We have too many pre-existing violations of
# this.
Comment on lines +1 to +6
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can send a separate PR to remove these later, just trying to control the size of the diff right now.

8 changes: 4 additions & 4 deletions bin/run_data_task.py
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,18 @@ def main():
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I've removed the executable bit from run_data_task.py. Ruff complains if a file is marked as executable that doesn't have a shebang line. It always seems to be run as python bin/run_data_task.py so the executable bit isn't needed


# Run the update in a transaction, so we roll back if it goes wrong
with request.db.bind.connect() as connection:
with request.db.bind.connect() as connection: # noqa: SIM117
with connection.begin():
for script in scripts:
if args.no_python and isinstance(script, PythonScript):
print(f"Skipping: {script}")
print(f"Skipping: {script}") # noqa: T201
continue

for step in script.execute(connection, dry_run=args.dry_run):
if args.dry_run:
print("Dry run!")
print("Dry run!") # noqa: T201

print(step.dump(indent=" ") + "\n")
print(step.dump(indent=" ") + "\n") # noqa: T201


if __name__ == "__main__":
Expand Down
19 changes: 10 additions & 9 deletions h/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,24 @@


def fetch_git_ref():
return subprocess.check_output(
["git", "rev-parse", "--short", "HEAD"], stderr=DEVNULL
return subprocess.check_output( # noqa: S603
["git", "rev-parse", "--short", "HEAD"], # noqa: S607
stderr=DEVNULL,
).strip()


def fetch_git_date(ref):
output = subprocess.check_output(["git", "show", "-s", "--format=%ct", ref])
return datetime.datetime.fromtimestamp(int(output))
output = subprocess.check_output(["git", "show", "-s", "--format=%ct", ref]) # noqa: S603, S607
return datetime.datetime.fromtimestamp(int(output)) # noqa: DTZ006


def fetch_git_dirty():
# Ensure git index is up-to-date first. This usually isn't necessary, but
# can be needed inside a docker container where the index is out of date.
subprocess.call(["git", "update-index", "-q", "--refresh"])
dirty_tree = bool(subprocess.call(["git", "diff-files", "--quiet"]))
subprocess.call(["git", "update-index", "-q", "--refresh"]) # noqa: S603, S607
dirty_tree = bool(subprocess.call(["git", "diff-files", "--quiet"])) # noqa: S603, S607
dirty_index = bool(
subprocess.call(["git", "diff-index", "--quiet", "--cached", "HEAD"])
subprocess.call(["git", "diff-index", "--quiet", "--cached", "HEAD"]) # noqa: S603, S607
)
return dirty_tree or dirty_index

Expand All @@ -45,11 +46,11 @@ def git_version():

def git_archive_version(): # pragma: no cover
ref = VERSION_GIT_REF
date = datetime.datetime.fromtimestamp(int(VERSION_GIT_DATE))
date = datetime.datetime.fromtimestamp(int(VERSION_GIT_DATE)) # noqa: DTZ006
return pep440_version(date, ref)


def pep440_version(date, ref, dirty=False):
def pep440_version(date, ref, dirty=False): # noqa: FBT002
"""Build a PEP440-compliant version number from the passed information."""
return f"{date.strftime('%Y%m%d')}+g{ref}{'.dirty' if dirty else ''}"

Expand Down
10 changes: 5 additions & 5 deletions h/accounts/schemas.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import codecs
import logging
from datetime import datetime, timedelta
from functools import lru_cache
from functools import cache

import colander
import deform
Expand All @@ -24,18 +24,18 @@
log = logging.getLogger(__name__)


@lru_cache(maxsize=None)
@cache
def get_blacklist():
# Try to load the blacklist file from disk. If, for whatever reason, we
# can't load the file, then don't crash out, just log a warning about
# the problem.
try:
with codecs.open("h/accounts/blacklist", encoding="utf-8") as handle:
blacklist = handle.readlines()
except (IOError, ValueError): # pragma: no cover
except (IOError, ValueError): # pragma: no cover # noqa: UP024
log.exception("unable to load blacklist")
blacklist = []
return set(line.strip().lower() for line in blacklist)
return set(line.strip().lower() for line in blacklist) # noqa: C401


def unique_email(node, value):
Expand Down Expand Up @@ -68,7 +68,7 @@ def unique_username(node, value):
)
# 31 days is an arbitrary time delta that should be more than enough
# time for all the previous user's data to be expunged.
.where(models.UserDeletion.requested_at > datetime.now() - timedelta(days=31))
.where(models.UserDeletion.requested_at > datetime.now() - timedelta(days=31)) # noqa: DTZ005
).first():
raise exc

Expand Down
8 changes: 4 additions & 4 deletions h/accounts/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ def validate_url(url):
parsed_url = urlparse("http://" + url)

if not re.match("https?", parsed_url.scheme):
raise ValueError('Links must have an "http" or "https" prefix')
raise ValueError('Links must have an "http" or "https" prefix') # noqa: EM101, TRY003

if not parsed_url.netloc:
raise ValueError("Links must include a domain name")
raise ValueError("Links must include a domain name") # noqa: EM101, TRY003

return parsed_url.geturl()

Expand All @@ -44,10 +44,10 @@ def validate_orcid(orcid):
orcid_regex = r"\A[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]\Z"

if not re.match(orcid_regex, orcid):
raise ValueError(f"The format of this ORCID is incorrect: {orcid}")
raise ValueError(f"The format of this ORCID is incorrect: {orcid}") # noqa: EM102, TRY003

if _orcid_checksum_digit(orcid[:-1]) != orcid[-1:]:
raise ValueError(f"{orcid} is not a valid ORCID")
raise ValueError(f"{orcid} is not a valid ORCID") # noqa: EM102, TRY003

return True

Expand Down
4 changes: 2 additions & 2 deletions h/activity/bucketing.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def next(self, annotation):
if timeframe.within_cutoff(annotation):
return timeframe

cutoff_time = datetime.datetime(
cutoff_time = datetime.datetime( # noqa: DTZ001
year=annotation.updated.year, month=annotation.updated.month, day=1
)
timeframe = Timeframe(annotation.updated.strftime("%b %Y"), cutoff_time)
Expand Down Expand Up @@ -179,4 +179,4 @@ def bucket(annotations):


def utcnow(): # pragma: no cover
return datetime.datetime.utcnow()
return datetime.datetime.utcnow() # noqa: DTZ003
2 changes: 1 addition & 1 deletion h/activity/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from h.services.annotation_read import AnnotationReadService


class ActivityResults(
class ActivityResults( # noqa: SLOT002
namedtuple("ActivityResults", ["total", "aggregations", "timeframes"]) # noqa: PYI024
):
pass
Expand Down
2 changes: 1 addition & 1 deletion h/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def includeme(config): # pragma: no cover

config.add_settings(
{
"tm.manager_hook": lambda request: transaction.TransactionManager(),
"tm.manager_hook": lambda request: transaction.TransactionManager(), # noqa: ARG005
"tm.annotate_user": False,
}
)
Expand Down
4 changes: 2 additions & 2 deletions h/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)


def bootstrap(app_url, dev=False):
def bootstrap(app_url, dev=False): # noqa: FBT002
"""
Bootstrap the application from the given arguments.

Expand All @@ -34,7 +34,7 @@ def bootstrap(app_url, dev=False):
if dev:
app_url = "http://localhost:5000"
else:
raise click.ClickException("the app URL must be set in production mode!")
raise click.ClickException("the app URL must be set in production mode!") # noqa: EM101, TRY003

config = "conf/development.ini" if dev else "conf/production.ini"

Expand Down
2 changes: 1 addition & 1 deletion h/cli/commands/authclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def authclient():
help="An allowable grant type",
)
@click.pass_context
def add(ctx, name, authority, type_, redirect_uri, grant_type):
def add(ctx, name, authority, type_, redirect_uri, grant_type): # noqa: PLR0913
"""Create a new OAuth client."""
request = ctx.obj["bootstrap"]()

Expand Down
14 changes: 7 additions & 7 deletions h/cli/commands/create_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ def create_annotations(ctx, number):
tm = request.tm

for _ in range(number):
created = updated = datetime.datetime(
year=random.randint(2015, 2020),
month=random.randint(1, 12),
day=random.randint(1, 27),
hour=random.randint(1, 12),
minute=random.randint(0, 59),
second=random.randint(0, 59),
created = updated = datetime.datetime( # noqa: DTZ001
year=random.randint(2015, 2020), # noqa: S311
month=random.randint(1, 12), # noqa: S311
day=random.randint(1, 27), # noqa: S311
hour=random.randint(1, 12), # noqa: S311
minute=random.randint(0, 59), # noqa: S311
second=random.randint(0, 59), # noqa: S311
)
db.add(
factories.Annotation.build(created=created, updated=updated, shared=True)
Expand Down
19 changes: 10 additions & 9 deletions h/cli/commands/devdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def create_all(self):
elif type_ == "restricted_group":
self.upsert_restricted_group(data_dict)
else:
raise RuntimeError(f"Unrecognized type: {type_}")
raise RuntimeError(f"Unrecognized type: {type_}") # noqa: EM102, TRY003

self.tm.commit()

Expand Down Expand Up @@ -113,7 +113,7 @@ def upsert_group(self, group_data, group_create_method):
creator = models.User.get_by_username(
self.db, group_data.pop("creator_username"), group_data["authority"]
)
assert creator
assert creator # noqa: S101

organization = (
self.db.query(models.Organization)
Expand Down Expand Up @@ -145,23 +145,24 @@ def setattrs(object_, attrs):
def devdata(ctx):
with tempfile.TemporaryDirectory() as tmpdirname:
# The directory that we'll clone the devdata git repo into.
git_dir = os.path.join(tmpdirname, "devdata")
git_dir = os.path.join(tmpdirname, "devdata") # noqa: PTH118

# Clone the private devdata repo from GitHub.
# This will fail if Git->GitHub HTTPS authentication isn't set up or if
# your GitHub account doesn't have access to the private repo.
subprocess.check_call(
["git", "clone", "https://github.com/hypothesis/devdata.git", git_dir]
subprocess.check_call( # noqa: S603
["git", "clone", "https://github.com/hypothesis/devdata.git", git_dir] # noqa: S607
)

# Copy environment variables file into place.
shutil.copyfile(
os.path.join(git_dir, "h", "devdata.env"),
os.path.join(pathlib.Path(h.__file__).parent.parent, ".devdata.env"),
os.path.join(git_dir, "h", "devdata.env"), # noqa: PTH118
os.path.join(pathlib.Path(h.__file__).parent.parent, ".devdata.env"), # noqa: PTH118
)

with open(
os.path.join(git_dir, "h", "devdata.json"), "r", encoding="utf8"
with open( # noqa: PTH123
os.path.join(git_dir, "h", "devdata.json"), # noqa: PTH118
encoding="utf8",
) as handle:
DevDataFactory(
ctx.obj["bootstrap"](),
Expand Down
2 changes: 1 addition & 1 deletion h/cli/commands/move_uri.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def move_uri(ctx, old, new):
)
answer = click.prompt(prompt, default="n", show_default=False)
if answer != "y":
print("Aborted")
print("Aborted") # noqa: T201
return

for annotation in annotations:
Expand Down
2 changes: 1 addition & 1 deletion h/cli/commands/normalize_uris.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from h.util import uri


class Window(namedtuple("Window", ["start", "end"])): # noqa: PYI024
class Window(namedtuple("Window", ["start", "end"])): # noqa: PYI024, SLOT002
pass


Expand Down
2 changes: 1 addition & 1 deletion h/cli/commands/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ def update_settings(ctx):
try:
config.update_index_settings(request.es)
except RuntimeError as exc:
raise click.ClickException(str(exc))
raise click.ClickException(str(exc)) # noqa: B904
6 changes: 3 additions & 3 deletions h/cli/commands/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def add(ctx, username, email, password, authority):
message = (
f"could not create user due to integrity constraint.\n\n{upstream_error}"
)
raise click.ClickException(message)
raise click.ClickException(message) # noqa: B904

click.echo(f"{username} created", err=True)

Expand Down Expand Up @@ -92,8 +92,8 @@ def password(ctx, username, authority, password):

user = models.User.get_by_username(request.db, username, authority)
if user is None:
raise click.ClickException(
f'no user with username "{username}" and authority "{authority}"'
raise click.ClickException( # noqa: TRY003
f'no user with username "{username}" and authority "{authority}"' # noqa: EM102
)

password_service.update_password(user, password)
Expand Down
2 changes: 1 addition & 1 deletion h/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
)


def configure(environ=None, settings=None):
def configure(environ=None, settings=None): # noqa: PLR0915
if environ is None: # pragma: no cover
environ = os.environ
if settings is None: # pragma: no cover
Expand Down
2 changes: 1 addition & 1 deletion h/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from sqlalchemy import text
from sqlalchemy.orm import declarative_base, sessionmaker

__all__ = ("Base", "Session", "pre_create", "post_create", "create_engine")
__all__ = ("Base", "Session", "create_engine", "post_create", "pre_create")

log = logging.getLogger(__name__)

Expand Down
Loading
Loading