Skip to content

Commit

Permalink
pylint cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
ukanga committed Jul 4, 2024
1 parent 1b6c8c3 commit 330b2c6
Show file tree
Hide file tree
Showing 96 changed files with 623 additions and 524 deletions.
13 changes: 1 addition & 12 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ load-plugins=pylint_django,pylint_celery
# Pickle collected data for later comparisons.
persistent=yes

# Resolve imports to .pyi stubs if available. May reduce no-member messages and
# increase not-an-iterable messages.
prefer-stubs=no

# Minimum Python version to use for version dependent checks. Will default to
# the version used to run pylint.
py-version=3.12
Expand Down Expand Up @@ -433,9 +429,7 @@ disable=raw-checker-failed,
suppressed-message,
useless-suppression,
deprecated-pragma,
use-symbolic-message-instead,
use-implicit-booleaness-not-comparison-to-string,
use-implicit-booleaness-not-comparison-to-zero
use-symbolic-message-instead

# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
Expand Down Expand Up @@ -473,11 +467,6 @@ max-nested-blocks=5
# printed.
never-returning-functions=sys.exit,argparse.parse_error

# Let 'consider-using-join' be raised when the separator to join on would be
# non-empty (resulting in expected fixes of the type: ``"- " + " -
# ".join(items)``)
suggest-join-with-non-empty-separator=yes


[REPORTS]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ def handle(self, *args, **options):
count += 1
total += 1

except ObjectDoesNotExist as e:
except ObjectDoesNotExist as error:
fail += 1
self.stdout.write(str(e), ending="\n")
self.stdout.write(str(error), ending="\n")
else:
# Get all the teams
for team in queryset_iterator(
Expand Down
4 changes: 3 additions & 1 deletion onadata/apps/api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,9 @@ def regenerate_form_instance_json(xform_id: int):
safe_delete(cache_key)


class ShareProjectBaseTask(app.Task):
class ShareProjectBaseTask(app.Task): # pylint: disable=too-few-public-methods
"""A Task base class for sharing a project."""

autoretry_for = (
DatabaseError,
ConnectionError,
Expand Down
15 changes: 7 additions & 8 deletions onadata/apps/api/viewsets/attachment_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def get_attachment_data(attachment, suffix):
if suffix in list(settings.THUMB_CONF):
image_url(attachment, suffix)
suffix = settings.THUMB_CONF.get(suffix).get("suffix")
f = default_storage.open(get_path(attachment.media_file.name, suffix))
return f.read()
media_file = default_storage.open(get_path(attachment.media_file.name, suffix))
return media_file.read()

return attachment.media_file.read()

Expand Down Expand Up @@ -77,13 +77,12 @@ def retrieve(self, request, *args, **kwargs):
suffix = request.query_params.get("suffix")
try:
data = get_attachment_data(self.object, suffix)
except IOError as e:
if str(e).startswith("File does not exist"):
raise Http404() from e
except IOError as error:
if str(error).startswith("File does not exist"):
raise Http404() from error

raise ParseError(e) from e
else:
return Response(data, content_type=self.object.mimetype)
raise ParseError(error) from error
return Response(data, content_type=self.object.mimetype)

filename = request.query_params.get("filename")
serializer = self.get_serializer(self.object)
Expand Down
8 changes: 5 additions & 3 deletions onadata/apps/api/viewsets/media_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ def retrieve(self, request, *args, **kwargs):
Redirect to final attachment url
param pk: the attachment id
query param filename: the filename of the associated attachment is required and has to match
query param suffix: (optional) - specify small | medium | large to return resized images.
query param filename: the filename of the attachment is required and must match
query param suffix: (optional) - specify small | medium | large to
return resized images.
return HttpResponseRedirect: redirects to final image url
"""
Expand Down Expand Up @@ -84,6 +85,7 @@ def retrieve(self, request, *args, **kwargs):

def list(self, request, *args, **kwargs):
"""
Action NOT IMPLEMENTED, only needed because of the automatic url routing in /api/v1/
Action NOT IMPLEMENTED.
It is only needed because of the automatic URL routing in /api/v1/
"""
return Response(data=[])
2 changes: 1 addition & 1 deletion onadata/apps/api/viewsets/messaging_stats_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class MessagingStatsViewSet(mixins.ListModelMixin, viewsets.GenericViewSet):
The endpoint accepts the following query parameters:
- `group_by`: field specifying whether to group events by `day`, `month` or `year`
- `group_by`: field to group events by `day`, `month` or `year`
- `target_type`: field to be used to determine the target
object type i.e xform, project
- `target_id`: field used to identify the target object
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/api/viewsets/user_profile_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ def change_password(self, request, *args, **kwargs): # noqa

try:
validate_password(new_password, user=user_profile.user)
except ValidationError as e:
except ValidationError as error:
return Response(
data={"errors": e.messages}, status=status.HTTP_400_BAD_REQUEST
data={"errors": error.messages}, status=status.HTTP_400_BAD_REQUEST
)

data = {"username": user_profile.user.username}
Expand Down
8 changes: 4 additions & 4 deletions onadata/apps/api/viewsets/v2/tableau_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ def unpack_repeat_data(repeat_data, flat_dict):
cleaned_data = []
for data_dict in repeat_data:
remove_keys = []
for k, v in data_dict.items():
if isinstance(v, list):
remove_keys.append(k)
flat_dict[k].extend(v)
for key, value in data_dict.items():
if isinstance(value, list):
remove_keys.append(key)
flat_dict[key].extend(value)
# pylint: disable=expression-not-assigned
[data_dict.pop(k) for k in remove_keys]
cleaned_data.append(data_dict)
Expand Down
2 changes: 1 addition & 1 deletion onadata/apps/logger/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from onadata.apps.logger.models import Project, XForm


class FilterByUserMixin:
class FilterByUserMixin: # pylint: disable=too-few-public-methods
"""Filter queryset by ``request.user``."""

# A user should only see forms/projects that belong to him.
Expand Down
8 changes: 4 additions & 4 deletions onadata/apps/logger/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ def get_registration_instance(self, custom_values=None):
# pylint: disable=protected-access
reg_instance._id = xform.id_string

for k, v in values.items():
reg_instance.answer(name=k, value=v)
for key, value in values.items():
reg_instance.answer(name=key, value=value)

instance_xml = reg_instance.to_xml()

Expand Down Expand Up @@ -198,8 +198,8 @@ def get_simple_instance(self, custom_values=None):
water_simple_survey = _load_simple_survey_object()
simple_survey = water_simple_survey.instantiate()

for k, v in values.items():
simple_survey.answer(name=k, value=v)
for key, value in values.items():
simple_survey.answer(name=key, value=value)

# setting the id_string so that it doesn't end up
# with the timestamp of the new survey object
Expand Down
10 changes: 5 additions & 5 deletions onadata/apps/logger/import_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ def django_file(path, field_name, content_type):
# http://groups.google.com/group/django-users/browse_thread/thread/
# 834f988876ff3c45/
# pylint: disable=consider-using-with
f = open(path, "rb")
a_file = open(path, "rb")
return InMemoryUploadedFile(
file=f,
file=a_file,
field_name=field_name,
name=f.name,
name=a_file.name,
content_type=content_type,
size=os.path.getsize(path),
charset=None,
Expand Down Expand Up @@ -142,8 +142,8 @@ def import_instances_from_zip(zipfile_path, user, status="zip"):
try:
with zipfile.ZipFile(zipfile_path) as zip_file:
zip_file.extractall(temp_directory)
except zipfile.BadZipfile as e:
errors = [f"{e}"]
except zipfile.BadZipfile as error:
errors = [f"{error}"]
return 0, 0, errors
else:
return import_instances_from_path(temp_directory, user, status)
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/logger/management/commands/add_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ def add_id(self, user):
instance.save()
count += 1
# pylint: disable=broad-except
except Exception as e:
except Exception as error:
failed += 1
self.stdout.write(str(e), ending="\n")
self.stdout.write(str(error), ending="\n")

self.stdout.write(
f"Syncing for account {user.username}. Done. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def handle(self, *args, **kwargs):
all_files = s3_storage.bucket.list()

num = 0
for i, f in enumerate(all_files):
f.set_acl(permission)
for i, a_file in enumerate(all_files):
a_file.set_acl(permission)
if i % 1000 == 0:
self.stdout.write(_(f"{i} file objects processed"))
num = i
Expand Down
19 changes: 9 additions & 10 deletions onadata/apps/logger/management/commands/create_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,23 @@ def handle(self, *args, **options): # noqa C901
"""Create a zip backup of a form and all its submissions."""
try:
output_file = args[0]
except IndexError as e:
except IndexError as error:
raise CommandError(
_("Provide the path to the zip file to backup to")
) from e
else:
output_file = os.path.realpath(output_file)
) from error
output_file = os.path.realpath(output_file)

try:
username = args[1]
except IndexError as e:
except IndexError as error:
raise CommandError(
_("You must provide the username to publish the form to.")
) from e
) from error
# make sure user exists
try:
user = get_user_model().objects.get(username=username)
except get_user_model().DoesNotExist as e:
raise CommandError(_(f"The user '{username}' does not exist.")) from e
except get_user_model().DoesNotExist as error:
raise CommandError(_(f"The user '{username}' does not exist.")) from error

try:
id_string = args[2]
Expand All @@ -51,8 +50,8 @@ def handle(self, *args, **options): # noqa C901
# make sure xform exists
try:
xform = XForm.objects.get(user=user, id_string=id_string)
except XForm.DoesNotExist as e:
except XForm.DoesNotExist as error:
raise CommandError(
_(f"The id_string '{id_string}' does not exist.")
) from e
) from error
create_zip_backup(output_file, user, xform)
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,19 @@ def handle(self, *args, **options):
username = options.get("username")
try:
user = User.objects.get(username=username)
except User.DoesNotExist as e:
raise CommandError(f"Error: username {username} does not exist") from e
except User.DoesNotExist as error:
raise CommandError(
f"Error: username {username} does not exist"
) from error
attachments_qs = attachments_qs.filter(instance__user=user)
if options.get("id_string"):
id_string = options.get("id_string")
try:
xform = XForm.objects.get(id_string=id_string)
except XForm.DoesNotExist as e:
except XForm.DoesNotExist as error:
raise CommandError(
f"Error: Form with id_string {id_string} does not exist"
) from e
) from error
attachments_qs = attachments_qs.filter(instance__xform=xform)
file_storage = get_storage_class(
"django.core.files.storage.FileSystemStorage"
Expand Down
6 changes: 4 additions & 2 deletions onadata/apps/logger/management/commands/import_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ def handle(self, *args, **kwargs):
"""Import a folder of XForms for ODK."""
path = args[0]
for form in glob.glob(os.path.join(path, "*")):
with open(form, encoding="utf-8") as f:
XForm.objects.get_or_create(xml=f.read(), downloadable=False)
with open(form, encoding="utf-8") as xform_xml_file:
XForm.objects.get_or_create(
xml=xform_xml_file.read(), downloadable=False
)
4 changes: 2 additions & 2 deletions onadata/apps/logger/management/commands/import_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def handle(self, *args, **kwargs):
username = args[1]
try:
user = get_user_model().objects.get(username=username)
except get_user_model().DoesNotExist as e:
raise CommandError(_(f"Invalid username {username}")) from e
except get_user_model().DoesNotExist as error:
raise CommandError(_(f"Invalid username {username}")) from error
debug = False
if debug:
self.stdout.write(_(f"[Importing XForm Instances from {path}]\n"))
Expand Down
29 changes: 18 additions & 11 deletions onadata/apps/logger/management/commands/move_media_to_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,29 @@ def handle(self, *args, **kwargs):
for cls, file_field, upload_to in classes_to_move:
self.stdout.write(_("Moving %(class)ss to s3...") % {"class": cls.__name__})
for i in cls.objects.all():
f = getattr(i, file_field)
old_filename = f.name
media_file = getattr(i, file_field)
old_filename = media_file.name
if (
f.name
and local_fs.exists(f.name)
and not s3_fs.exists(upload_to(i, f.name))
old_filename
and local_fs.exists(old_filename)
and not s3_fs.exists(upload_to(i, old_filename))
):
f.save(local_fs.path(f.name), local_fs.open(local_fs.path(f.name)))
media_file.name.save(
local_fs.path(old_filename),
local_fs.open(local_fs.path(old_filename)),
)
self.stdout.write(
_("\t+ '%(fname)s'\n\t---> '%(url)s'")
% {"fname": local_fs.path(old_filename), "url": f.url}
% {
"fname": local_fs.path(old_filename),
"url": media_file.url,
}
)
else:
exists_locally = local_fs.exists(f.name)
exists_s3 = not s3_fs.exists(upload_to(i, f.name))
exists_locally = local_fs.exists(old_filename)
exists_s3 = not s3_fs.exists(upload_to(i, old_filename))
self.stderr.write(
f"\t- (f.name={f.name}, fs.exists(f.name)={exists_locally},"
f" not s3.exist s3upload_to(i, f.name))={exists_s3})"
f"\t- (old_filename={old_filename}, "
f"fs.exists(old_filename)={exists_locally},"
f" not s3.exist s3upload_to(i, old_filename))={exists_s3})"
)
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ def handle(self, *args, **kwargs):
xform.instances_with_geopoints = has_geo
xform.save()
# pylint: disable=broad-except
except Exception as e:
self.stderr.write(e)
except Exception as error:
self.stderr.write(error)
else:
count += 1
self.stdout.write(f"{count} of {total} forms processed.")
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def handle(self, *args, **kwargs):
xform.instances_with_osm = True
xform.save()
# pylint: disable=broad-except
except Exception as e:
self.stderr.write(e)
except Exception as error:
self.stderr.write(error)
else:
count += 1

Expand Down
10 changes: 6 additions & 4 deletions onadata/apps/logger/management/commands/update_xform_uuids.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def handle(self, *args, **kwargs):
raise CommandError("You must provide a path to the csv file")
# try open the file
try:
with open(kwargs.get("file"), "r", encoding="utf-8") as f:
lines = csv.reader(f)
with open(kwargs.get("file"), "r", encoding="utf-8") as csv_file:
lines = csv.reader(csv_file)
i = 0
for line in lines:
try:
Expand All @@ -55,5 +55,7 @@ def handle(self, *args, **kwargs):
else:
i += 1
self.stdout.write(f"Updated {i} rows")
except IOError as e:
raise CommandError(f"file {kwargs.get('file')} could not be open") from e
except IOError as error:
raise CommandError(
f"file {kwargs.get('file')} could not be open"
) from error
Loading

0 comments on commit 330b2c6

Please sign in to comment.