Merge pull request #1184 from bookwyrm-social/pylint

Adds pylint workflow

.github/workflows/black.yml

@@ -1,4 +1,4 @@
-name: Lint Python
+name: Python Formatting (run ./bw-dev black to fix)
 
 on: [push, pull_request]
 

.github/workflows/pylint.yml

@@ -0,0 +1,24 @@
+name: Pylint
+
+on: [push, pull_request]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python 3.9
+      uses: actions/setup-python@v2
+      with:
+        python-version: 3.9
+    - name: Install Dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+        pip install pylint
+    - name: Analysing the code with pylint
+      run: |
+        pylint bookwyrm/ --ignore=migrations,tests --disable=E1101,E1135,E1136,R0903,R0901,R0902,W0707,W0511,W0406,R0401,R0801
+

bookwyrm/activitypub/base_activity.py

@@ -37,6 +37,7 @@ class Mention(Link):
 
 
 @dataclass
+# pylint: disable=invalid-name
 class Signature:
     """public key block"""
 
@@ -56,11 +57,11 @@ def naive_parse(activity_objects, activity_json, serializer=None):
         activity_type = activity_json.get("type")
         try:
             serializer = activity_objects[activity_type]
-        except KeyError as e:
+        except KeyError as err:
             # we know this exists and that we can't handle it
             if activity_type in ["Question"]:
                 return None
-            raise ActivitySerializerError(e)
+            raise ActivitySerializerError(err)
 
     return serializer(activity_objects=activity_objects, **activity_json)
 

bookwyrm/activitypub/book.py

@@ -6,6 +6,7 @@ from .base_activity import ActivityObject
 from .image import Document
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class BookData(ActivityObject):
     """shared fields for all book data and authors"""
@@ -18,6 +19,7 @@ class BookData(ActivityObject):
     lastEditedBy: str = None
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Book(BookData):
     """serializes an edition or work, abstract"""
@@ -40,6 +42,7 @@ class Book(BookData):
     type: str = "Book"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Edition(Book):
     """Edition instance of a book object"""
@@ -57,6 +60,7 @@ class Edition(Book):
     type: str = "Edition"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Work(Book):
     """work instance of a book object"""
@@ -66,6 +70,7 @@ class Work(Book):
     type: str = "Work"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Author(BookData):
     """author of a book"""

bookwyrm/activitypub/note.py

@@ -19,6 +19,7 @@ class Tombstone(ActivityObject):
         return model.find_existing_by_remote_id(self.id)
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Note(ActivityObject):
     """Note activity"""
@@ -52,6 +53,7 @@ class GeneratedNote(Note):
     type: str = "GeneratedNote"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Comment(Note):
     """like a note but with a book"""

bookwyrm/activitypub/ordered_collection.py

@@ -5,6 +5,7 @@ from typing import List
 from .base_activity import ActivityObject
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class OrderedCollection(ActivityObject):
     """structure of an ordered collection activity"""
@@ -17,6 +18,7 @@ class OrderedCollection(ActivityObject):
     type: str = "OrderedCollection"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class OrderedCollectionPrivate(OrderedCollection):
     """an ordered collection with privacy settings"""
@@ -41,6 +43,7 @@ class BookList(OrderedCollectionPrivate):
     type: str = "BookList"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class OrderedCollectionPage(ActivityObject):
     """structure of an ordered collection activity"""

bookwyrm/activitypub/person.py

@@ -6,6 +6,7 @@ from .base_activity import ActivityObject
 from .image import Image
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class PublicKey(ActivityObject):
     """public key block"""
@@ -15,6 +16,7 @@ class PublicKey(ActivityObject):
     type: str = "PublicKey"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Person(ActivityObject):
     """actor activitypub json"""

bookwyrm/activitypub/response.py

@@ -1,3 +1,4 @@
+""" ActivityPub-specific json response wrapper """
 from django.http import JsonResponse
 
 from .base_activity import ActivityEncoder

bookwyrm/activitypub/verbs.py

@@ -22,6 +22,7 @@ class Verb(ActivityObject):
             self.object.to_model()
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Create(Verb):
     """Create activity"""
@@ -32,6 +33,7 @@ class Create(Verb):
     type: str = "Create"
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Delete(Verb):
     """Create activity"""
@@ -57,6 +59,7 @@ class Delete(Verb):
         # if we can't find it, we don't need to delete it because we don't have it
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Update(Verb):
     """Update activity"""
@@ -192,6 +195,7 @@ class Like(Verb):
         self.to_model()
 
 
+# pylint: disable=invalid-name
 @dataclass(init=False)
 class Announce(Verb):
     """boosting a status"""

bookwyrm/connectors/abstract_connector.py

@@ -127,8 +127,8 @@ class AbstractConnector(AbstractMinimalConnector):
             edition_data = data
             try:
                 work_data = self.get_work_from_edition_data(data)
-            except (KeyError, ConnectorException) as e:
+            except (KeyError, ConnectorException) as err:
-                logger.exception(e)
+                logger.exception(err)
                 work_data = data
 
         if not work_data or not edition_data:
@@ -237,16 +237,16 @@ def get_data(url, params=None, timeout=10):
             },
             timeout=timeout,
         )
-    except (RequestError, SSLError, ConnectionError) as e:
+    except (RequestError, SSLError, ConnectionError) as err:
-        logger.exception(e)
+        logger.exception(err)
         raise ConnectorException()
 
     if not resp.ok:
         raise ConnectorException()
     try:
         data = resp.json()
-    except ValueError as e:
+    except ValueError as err:
-        logger.exception(e)
+        logger.exception(err)
         raise ConnectorException()
 
     return data
@@ -262,8 +262,8 @@ def get_image(url, timeout=10):
             },
             timeout=timeout,
         )
-    except (RequestError, SSLError) as e:
+    except (RequestError, SSLError) as err:
-        logger.exception(e)
+        logger.exception(err)
         return None
     if not resp.ok:
         return None

bookwyrm/connectors/connector_manager.py

@@ -38,17 +38,17 @@ def search(query, min_confidence=0.1, return_first=False):
             # Search on ISBN
             try:
                 result_set = connector.isbn_search(isbn)
-            except Exception as e:  # pylint: disable=broad-except
+            except Exception as err:  # pylint: disable=broad-except
-                logger.exception(e)
+                logger.exception(err)
                 # if this fails, we can still try regular search
 
         # if no isbn search results, we fallback to generic search
         if not result_set:
             try:
                 result_set = connector.search(query, min_confidence=min_confidence)
-            except Exception as e:  # pylint: disable=broad-except
+            except Exception as err:  # pylint: disable=broad-except
                 # we don't want *any* error to crash the whole search page
-                logger.exception(e)
+                logger.exception(err)
                 continue
 
         if return_first and result_set:

bookwyrm/connectors/inventaire.py

@@ -74,7 +74,7 @@ class Connector(AbstractConnector):
             **{k: data.get(k) for k in ["uri", "image", "labels", "sitelinks"]},
         }
 
-    def search(self, query, min_confidence=None):
+    def search(self, query, min_confidence=None):  # pylint: disable=arguments-differ
         """overrides default search function with confidence ranking"""
         results = super().search(query)
         if min_confidence:

bookwyrm/connectors/self_connector.py

@@ -114,6 +114,7 @@ class Connector(AbstractConnector):
 
 def search_identifiers(query, *filters):
     """tries remote_id, isbn; defined as dedupe fields on the model"""
+    # pylint: disable=W0212
     or_filters = [
         {f.name: query}
         for f in models.Edition._meta.get_fields()

bookwyrm/forms.py

@@ -22,6 +22,7 @@ class CustomForm(ModelForm):
         css_classes["number"] = "input"
         css_classes["checkbox"] = "checkbox"
         css_classes["textarea"] = "textarea"
+        # pylint: disable=super-with-arguments
         super(CustomForm, self).__init__(*args, **kwargs)
         for visible in self.visible_fields():
             if hasattr(visible.field.widget, "input_type"):
@@ -181,8 +182,6 @@ class EditionForm(CustomForm):
             "authors",
             "parent_work",
             "shelves",
-            "subjects",  # TODO
-            "subject_places",  # TODO
             "connector",
         ]
 

bookwyrm/importers/importer.py

@@ -67,8 +67,8 @@ def import_data(source, job_id):
         for item in job.items.all():
             try:
                 item.resolve()
-            except Exception as e:  # pylint: disable=broad-except
+            except Exception as err:  # pylint: disable=broad-except
-                logger.exception(e)
+                logger.exception(err)
                 item.fail_reason = "Error loading book"
                 item.save()
                 continue

bookwyrm/management/commands/generate_preview_images.py

@@ -1,12 +1,13 @@
 """ Generate preview images """
-import sys
-
 from django.core.management.base import BaseCommand
 
-from bookwyrm import activitystreams, models, settings, preview_images
+from bookwyrm import models, preview_images
 
 
+# pylint: disable=line-too-long
 class Command(BaseCommand):
+    """Creates previews for existing objects"""
+
     help = "Generate preview images"
 
     def add_arguments(self, parser):

bookwyrm/models/book.py

@@ -8,8 +8,7 @@ from model_utils.managers import InheritanceManager
 
 from bookwyrm import activitypub
 from bookwyrm.preview_images import generate_edition_preview_image_task
-from bookwyrm.settings import DOMAIN, DEFAULT_LANGUAGE
+from bookwyrm.settings import DOMAIN, DEFAULT_LANGUAGE, ENABLE_PREVIEW_IMAGES
-from bookwyrm.tasks import app
 
 from .activitypub_mixin import OrderedCollectionPageMixin, ObjectMixin
 from .base_model import BookWyrmModel
@@ -303,9 +302,12 @@ def isbn_13_to_10(isbn_13):
     return converted + str(checkdigit)
 
 
-@receiver(models.signals.post_save, sender=Edition)
 # pylint: disable=unused-argument
+@receiver(models.signals.post_save, sender=Edition)
 def preview_image(instance, *args, **kwargs):
+    """create preview image on book create"""
+    if not ENABLE_PREVIEW_IMAGES:
+        return
     changed_fields = {}
     if instance.field_tracker:
         changed_fields = instance.field_tracker.changed()

bookwyrm/models/fields.py

@@ -202,6 +202,7 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField):
             *args, max_length=255, choices=PrivacyLevels.choices, default="public"
         )
 
+    # pylint: disable=invalid-name
     def set_field_from_activity(self, instance, data):
         to = data.to
         cc = data.cc
@@ -220,6 +221,7 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField):
         if hasattr(instance, "mention_users"):
             mentions = [u.remote_id for u in instance.mention_users.all()]
         # this is a link to the followers list
+        # pylint: disable=protected-access
         followers = instance.user.__class__._meta.get_field(
             "followers"
         ).field_to_activity(instance.user.followers)

bookwyrm/models/list.py

@@ -93,7 +93,8 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
             )
 
     class Meta:
-        # A book may only be placed into a list once, and each order in the list may be used only
+        """A book may only be placed into a list once,
-        # once
+        and each order in the list may be used only once"""
+
         unique_together = (("book", "book_list"), ("order", "book_list"))
         ordering = ("-created_date",)

bookwyrm/models/relationship.py

@@ -99,7 +99,7 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship):
     status = "follow_request"
     activity_serializer = activitypub.Follow
 
-    def save(self, *args, broadcast=True, **kwargs):
+    def save(self, *args, broadcast=True, **kwargs):  # pylint: disable=arguments-differ
         """make sure the follow or block relationship doesn't already exist"""
         # if there's a request for a follow that already exists, accept it
         # without changing the local database state

bookwyrm/models/site.py

@@ -9,8 +9,7 @@ from django.utils import timezone
 from model_utils import FieldTracker
 
 from bookwyrm.preview_images import generate_site_preview_image_task
-from bookwyrm.settings import DOMAIN
+from bookwyrm.settings import DOMAIN, ENABLE_PREVIEW_IMAGES
-from bookwyrm.tasks import app
 from .base_model import BookWyrmModel
 from .user import User
 
@@ -130,9 +129,12 @@ class PasswordReset(models.Model):
         return "https://{}/password-reset/{}".format(DOMAIN, self.code)
 
 
-@receiver(models.signals.post_save, sender=SiteSettings)
 # pylint: disable=unused-argument
+@receiver(models.signals.post_save, sender=SiteSettings)
 def preview_image(instance, *args, **kwargs):
+    """Update image preview for the default site image"""
+    if not ENABLE_PREVIEW_IMAGES:
+        return
     changed_fields = instance.field_tracker.changed()
 
     if len(changed_fields) > 0:

bookwyrm/models/status.py

@@ -13,6 +13,7 @@ from model_utils.managers import InheritanceManager
 
 from bookwyrm import activitypub
 from bookwyrm.preview_images import generate_edition_preview_image_task
+from bookwyrm.settings import ENABLE_PREVIEW_IMAGES
 from .activitypub_mixin import ActivitypubMixin, ActivityMixin
 from .activitypub_mixin import OrderedCollectionPageMixin
 from .base_model import BookWyrmModel
@@ -405,12 +406,15 @@ class Boost(ActivityMixin, Status):
     #     unique_together = ('user', 'boosted_status')
 
 
-@receiver(models.signals.post_save)
 # pylint: disable=unused-argument
+@receiver(models.signals.post_save)
 def preview_image(instance, sender, *args, **kwargs):
-    if sender in (Review, ReviewRating):
+    """Updates book previews if the rating has changed"""
-        changed_fields = instance.field_tracker.changed()
+    if not ENABLE_PREVIEW_IMAGES or sender not in (Review, ReviewRating):
+        return
 
-        if len(changed_fields) > 0:
+    changed_fields = instance.field_tracker.changed()
-            edition = instance.book
+
-            generate_edition_preview_image_task.delay(edition.id)
+    if len(changed_fields) > 0:
+        edition = instance.book
+        generate_edition_preview_image_task.delay(edition.id)

bookwyrm/models/user.py

@@ -17,7 +17,7 @@ from bookwyrm.connectors import get_data, ConnectorException
 from bookwyrm.models.shelf import Shelf
 from bookwyrm.models.status import Status, Review
 from bookwyrm.preview_images import generate_user_preview_image_task
-from bookwyrm.settings import DOMAIN
+from bookwyrm.settings import DOMAIN, ENABLE_PREVIEW_IMAGES
 from bookwyrm.signatures import create_key_pair
 from bookwyrm.tasks import app
 from bookwyrm.utils import regex
@@ -239,7 +239,7 @@ class User(OrderedCollectionPageMixin, AbstractUser):
     def save(self, *args, **kwargs):
         """populate fields for new local users"""
         created = not bool(self.id)
-        if not self.local and not re.match(regex.full_username, self.username):
+        if not self.local and not re.match(regex.FULL_USERNAME, self.username):
             # generate a username that uses the domain (webfinger format)
             actor_parts = urlparse(self.remote_id)
             self.username = "%s@%s" % (self.username, actor_parts.netloc)
@@ -363,7 +363,7 @@ class AnnualGoal(BookWyrmModel):
 
     def get_remote_id(self):
         """put the year in the path"""
-        return "%s/goal/%d" % (self.user.remote_id, self.year)
+        return "{:s}/goal/{:d}".format(self.user.remote_id, self.year)
 
     @property
     def books(self):
@@ -452,9 +452,12 @@ def get_remote_reviews(outbox):
         activitypub.Review(**activity).to_model()
 
 
-@receiver(models.signals.post_save, sender=User)
 # pylint: disable=unused-argument
+@receiver(models.signals.post_save, sender=User)
 def preview_image(instance, *args, **kwargs):
+    """create preview images when user is updated"""
+    if not ENABLE_PREVIEW_IMAGES:
+        return
     changed_fields = instance.field_tracker.changed()
 
     if len(changed_fields) > 0:

bookwyrm/preview_images.py

@@ -1,13 +1,14 @@
-import colorsys
+""" Generate social media preview images for twitter/mastodon/etc """
 import math
 import os
 import textwrap
-
-from colorthief import ColorThief
 from io import BytesIO
-from PIL import Image, ImageDraw, ImageFont, ImageOps, ImageColor
 from uuid import uuid4
 
+import colorsys
+from colorthief import ColorThief
+from PIL import Image, ImageDraw, ImageFont, ImageOps, ImageColor
+
 from django.core.files.base import ContentFile
 from django.core.files.uploadedfile import InMemoryUploadedFile
 from django.db.models import Avg
@@ -31,6 +32,7 @@ font_dir = os.path.join(settings.STATIC_ROOT, "fonts/public_sans")
 
 
 def get_font(font_name, size=28):
+    """Loads custom font"""
     if font_name == "light":
         font_path = os.path.join(font_dir, "PublicSans-Light.ttf")
     if font_name == "regular":
@@ -47,6 +49,7 @@ def get_font(font_name, size=28):
 
 
 def generate_texts_layer(texts, content_width):
+    """Adds text for images"""
     font_text_zero = get_font("bold", size=20)
     font_text_one = get_font("bold", size=48)
     font_text_two = get_font("bold", size=40)
@@ -66,7 +69,7 @@ def generate_texts_layer(texts, content_width):
 
         try:
             text_y = text_y + font_text_zero.getsize_multiline(text_zero)[1] + 16
-        except:
+        except (AttributeError, IndexError):
             text_y = text_y + 26
 
     if "text_one" in texts and texts["text_one"]:
@@ -78,7 +81,7 @@ def generate_texts_layer(texts, content_width):
 
         try:
             text_y = text_y + font_text_one.getsize_multiline(text_one)[1] + 16
-        except:
+        except (AttributeError, IndexError):
             text_y = text_y + 26
 
     if "text_two" in texts and texts["text_two"]:
@@ -90,7 +93,7 @@ def generate_texts_layer(texts, content_width):
 
         try:
             text_y = text_y + font_text_one.getsize_multiline(text_two)[1] + 16
-        except:
+        except (AttributeError, IndexError):
             text_y = text_y + 26
 
     if "text_three" in texts and texts["text_three"]:
@@ -105,6 +108,7 @@ def generate_texts_layer(texts, content_width):
 
 
 def generate_instance_layer(content_width):
+    """Places components for instance preview"""
     font_instance = get_font("light", size=28)
 
     site = models.SiteSettings.objects.get()
@@ -145,6 +149,7 @@ def generate_instance_layer(content_width):
 
 
 def generate_rating_layer(rating, content_width):
+    """Places components for rating preview"""
     try:
         icon_star_full = Image.open(
             os.path.join(settings.STATIC_ROOT, "images/icons/star-full.png")
@@ -155,47 +160,46 @@ def generate_rating_layer(rating, content_width):
         icon_star_half = Image.open(
             os.path.join(settings.STATIC_ROOT, "images/icons/star-half.png")
         )
-
+    except FileNotFoundError:
-        icon_size = 64
-        icon_margin = 10
-
-        rating_layer_base = Image.new(
-            "RGBA", (content_width, icon_size), color=TRANSPARENT_COLOR
-        )
-        rating_layer_color = Image.new(
-            "RGBA", (content_width, icon_size), color=TEXT_COLOR
-        )
-        rating_layer_mask = Image.new(
-            "RGBA", (content_width, icon_size), color=TRANSPARENT_COLOR
-        )
-
-        position_x = 0
-
-        for r in range(math.floor(rating)):
-            rating_layer_mask.alpha_composite(icon_star_full, (position_x, 0))
-            position_x = position_x + icon_size + icon_margin
-
-        if math.floor(rating) != math.ceil(rating):
-            rating_layer_mask.alpha_composite(icon_star_half, (position_x, 0))
-            position_x = position_x + icon_size + icon_margin
-
-        for r in range(5 - math.ceil(rating)):
-            rating_layer_mask.alpha_composite(icon_star_empty, (position_x, 0))
-            position_x = position_x + icon_size + icon_margin
-
-        rating_layer_mask = rating_layer_mask.getchannel("A")
-        rating_layer_mask = ImageOps.invert(rating_layer_mask)
-
-        rating_layer_composite = Image.composite(
-            rating_layer_base, rating_layer_color, rating_layer_mask
-        )
-
-        return rating_layer_composite
-    except:
         return None
 
+    icon_size = 64
+    icon_margin = 10
+
+    rating_layer_base = Image.new(
+        "RGBA", (content_width, icon_size), color=TRANSPARENT_COLOR
+    )
+    rating_layer_color = Image.new("RGBA", (content_width, icon_size), color=TEXT_COLOR)
+    rating_layer_mask = Image.new(
+        "RGBA", (content_width, icon_size), color=TRANSPARENT_COLOR
+    )
+
+    position_x = 0
+
+    for _ in range(math.floor(rating)):
+        rating_layer_mask.alpha_composite(icon_star_full, (position_x, 0))
+        position_x = position_x + icon_size + icon_margin
+
+    if math.floor(rating) != math.ceil(rating):
+        rating_layer_mask.alpha_composite(icon_star_half, (position_x, 0))
+        position_x = position_x + icon_size + icon_margin
+
+    for _ in range(5 - math.ceil(rating)):
+        rating_layer_mask.alpha_composite(icon_star_empty, (position_x, 0))
+        position_x = position_x + icon_size + icon_margin
+
+    rating_layer_mask = rating_layer_mask.getchannel("A")
+    rating_layer_mask = ImageOps.invert(rating_layer_mask)
+
+    rating_layer_composite = Image.composite(
+        rating_layer_base, rating_layer_color, rating_layer_mask
+    )
+
+    return rating_layer_composite
+
 
 def generate_default_inner_img():
+    """Adds cover image"""
     font_cover = get_font("light", size=28)
 
     default_cover = Image.new(
@@ -214,16 +218,19 @@ def generate_default_inner_img():
     return default_cover
 
 
+# pylint: disable=too-many-locals
 def generate_preview_image(
-    texts={}, picture=None, rating=None, show_instance_layer=True
+    texts=None, picture=None, rating=None, show_instance_layer=True
 ):
+    """Puts everything together"""
+    texts = texts or {}
     # Cover
     try:
         inner_img_layer = Image.open(picture)
         inner_img_layer.thumbnail((inner_img_width, inner_img_height), Image.ANTIALIAS)
         color_thief = ColorThief(picture)
         dominant_color = color_thief.get_color(quality=1)
-    except:
+    except:  # pylint: disable=bare-except
         inner_img_layer = generate_default_inner_img()
         dominant_color = ImageColor.getrgb(DEFAULT_COVER_COLOR)
 
@@ -246,7 +253,7 @@ def generate_preview_image(
             image_bg_color_hls[2],
         )
         image_bg_color = tuple(
-            [math.ceil(x * 255) for x in colorsys.hls_to_rgb(*image_bg_color_hls)]
+            math.ceil(x * 255) for x in colorsys.hls_to_rgb(*image_bg_color_hls)
         )
     else:
         image_bg_color = BG_COLOR
@@ -292,8 +299,7 @@ def generate_preview_image(
         # Remove Instance Layer from centering calculations
         contents_y = contents_y - math.floor((instance_layer.height + gutter) / 2)
 
-    if contents_y < margin:
+    contents_y = max(contents_y, margin)
-        contents_y = margin
 
     # Composite layers
     img.paste(
@@ -305,108 +311,114 @@ def generate_preview_image(
 
 
 def save_and_cleanup(image, instance=None):
-    if isinstance(instance, (models.Book, models.User, models.SiteSettings)):
+    """Save and close the file"""
-        file_name = "%s-%s.jpg" % (str(instance.id), str(uuid4()))
+    if not isinstance(instance, (models.Book, models.User, models.SiteSettings)):
-        image_buffer = BytesIO()
-
-        try:
-            try:
-                old_path = instance.preview_image.path
-            except ValueError:
-                old_path = ""
-
-            # Save
-            image.save(image_buffer, format="jpeg", quality=75)
-
-            instance.preview_image = InMemoryUploadedFile(
-                ContentFile(image_buffer.getvalue()),
-                "preview_image",
-                file_name,
-                "image/jpg",
-                image_buffer.tell(),
-                None,
-            )
-
-            save_without_broadcast = isinstance(instance, (models.Book, models.User))
-            if save_without_broadcast:
-                result = instance.save(broadcast=False)
-            else:
-                instance.save()
-
-            # Clean up old file after saving
-            if os.path.exists(old_path):
-                os.remove(old_path)
-
-        finally:
-            image_buffer.close()
-            return True
-    else:
         return False
+    file_name = "%s-%s.jpg" % (str(instance.id), str(uuid4()))
+    image_buffer = BytesIO()
+
+    try:
+        try:
+            old_path = instance.preview_image.path
+        except ValueError:
+            old_path = ""
+
+        # Save
+        image.save(image_buffer, format="jpeg", quality=75)
+
+        instance.preview_image = InMemoryUploadedFile(
+            ContentFile(image_buffer.getvalue()),
+            "preview_image",
+            file_name,
+            "image/jpg",
+            image_buffer.tell(),
+            None,
+        )
+
+        save_without_broadcast = isinstance(instance, (models.Book, models.User))
+        if save_without_broadcast:
+            instance.save(broadcast=False)
+        else:
+            instance.save()
+
+        # Clean up old file after saving
+        if os.path.exists(old_path):
+            os.remove(old_path)
+
+    finally:
+        image_buffer.close()
+    return True
 
 
+# pylint: disable=invalid-name
 @app.task
 def generate_site_preview_image_task():
     """generate preview_image for the website"""
-    if settings.ENABLE_PREVIEW_IMAGES == True:
+    if not settings.ENABLE_PREVIEW_IMAGES:
-        site = models.SiteSettings.objects.get()
+        return
 
-        if site.logo:
+    site = models.SiteSettings.objects.get()
-            logo = site.logo
-        else:
-            logo = os.path.join(settings.STATIC_ROOT, "images/logo.png")
 
-        texts = {
+    if site.logo:
-            "text_zero": settings.DOMAIN,
+        logo = site.logo
-            "text_one": site.name,
+    else:
-            "text_three": site.instance_tagline,
+        logo = os.path.join(settings.STATIC_ROOT, "images/logo.png")
-        }
 
-        image = generate_preview_image(
+    texts = {
-            texts=texts, picture=logo, show_instance_layer=False
+        "text_zero": settings.DOMAIN,
-        )
+        "text_one": site.name,
+        "text_three": site.instance_tagline,
+    }
 
-        save_and_cleanup(image, instance=site)
+    image = generate_preview_image(texts=texts, picture=logo, show_instance_layer=False)
+
+    save_and_cleanup(image, instance=site)
 
 
+# pylint: disable=invalid-name
 @app.task
 def generate_edition_preview_image_task(book_id):
     """generate preview_image for a book"""
-    if settings.ENABLE_PREVIEW_IMAGES == True:
+    if not settings.ENABLE_PREVIEW_IMAGES:
-        book = models.Book.objects.select_subclasses().get(id=book_id)
+        return
 
-        rating = models.Review.objects.filter(
+    book = models.Book.objects.select_subclasses().get(id=book_id)
-            privacy="public",
-            deleted=False,
-            book__in=[book_id],
-        ).aggregate(Avg("rating"))["rating__avg"]
 
-        texts = {
+    rating = models.Review.objects.filter(
-            "text_one": book.title,
+        privacy="public",
-            "text_two": book.subtitle,
+        deleted=False,
-            "text_three": book.author_text,
+        book__in=[book_id],
-        }
+    ).aggregate(Avg("rating"))["rating__avg"]
 
-        image = generate_preview_image(texts=texts, picture=book.cover, rating=rating)
+    texts = {
+        "text_one": book.title,
+        "text_two": book.subtitle,
+        "text_three": book.author_text,
+    }
 
-        save_and_cleanup(image, instance=book)
+    image = generate_preview_image(texts=texts, picture=book.cover, rating=rating)
+
+    save_and_cleanup(image, instance=book)
 
 
 @app.task
 def generate_user_preview_image_task(user_id):
     """generate preview_image for a book"""
-    if settings.ENABLE_PREVIEW_IMAGES == True:
+    if not settings.ENABLE_PREVIEW_IMAGES:
-        user = models.User.objects.get(id=user_id)
+        return
 
-        texts = {
+    user = models.User.objects.get(id=user_id)
-            "text_one": user.display_name,
-            "text_three": "@{}@{}".format(user.localname, settings.DOMAIN),
-        }
 
-        if user.avatar:
+    texts = {
-            avatar = user.avatar
+        "text_one": user.display_name,
-        else:
+        "text_three": "@{}@{}".format(user.localname, settings.DOMAIN),
-            avatar = os.path.join(settings.STATIC_ROOT, "images/default_avi.jpg")
+    }
 
-        image = generate_preview_image(texts=texts, picture=avatar)
+    if user.avatar:
+        avatar = user.avatar
+    else:
+        avatar = os.path.join(settings.STATIC_ROOT, "images/default_avi.jpg")
 
-        save_and_cleanup(image, instance=user)
+    image = generate_preview_image(texts=texts, picture=avatar)
+
+    save_and_cleanup(image, instance=user)

bookwyrm/signatures.py

@@ -73,6 +73,7 @@ class Signature:
         self.headers = headers
         self.signature = signature
 
+    # pylint: disable=invalid-name
     @classmethod
     def parse(cls, request):
         """extract and parse a signature from an http request"""

bookwyrm/tests/test_preview_images.py

@@ -18,9 +18,9 @@ from bookwyrm.preview_images import (
     save_and_cleanup,
 )
 
-import logging
-
 
+# pylint: disable=unused-argument
+# pylint: disable=missing-function-docstring
 class PreviewImages(TestCase):
     """every response to a get request, html or json"""
 

bookwyrm/timezone_middleware.py

@@ -1,9 +1,12 @@
+""" Makes the app aware of the users timezone """
 import pytz
 
 from django.utils import timezone
 
 
 class TimezoneMiddleware:
+    """Determine the timezone based on the request"""
+
     def __init__(self, get_response):
         self.get_response = get_response
 

bookwyrm/urls.py

@@ -7,8 +7,8 @@ from django.views.generic.base import TemplateView
 from bookwyrm import settings, views
 from bookwyrm.utils import regex
 
-user_path = r"^user/(?P<username>%s)" % regex.username
+USER_PATH = r"^user/(?P<username>%s)" % regex.USERNAME
-local_user_path = r"^user/(?P<username>%s)" % regex.localname
+LOCAL_USER_PATH = r"^user/(?P<username>%s)" % regex.LOCALNAME
 
 status_types = [
     "status",
@@ -19,9 +19,9 @@ status_types = [
     "boost",
     "generatednote",
 ]
-status_path = r"%s/(%s)/(?P<status_id>\d+)" % (user_path, "|".join(status_types))
+STATUS_PATH = r"%s/(%s)/(?P<status_id>\d+)" % (USER_PATH, "|".join(status_types))
 
-book_path = r"^book/(?P<book_id>\d+)"
+BOOK_PATH = r"^book/(?P<book_id>\d+)"
 
 urlpatterns = [
     path("admin/", admin.site.urls),
@@ -31,8 +31,8 @@ urlpatterns = [
     ),
     # federation endpoints
     re_path(r"^inbox/?$", views.Inbox.as_view()),
-    re_path(r"%s/inbox/?$" % local_user_path, views.Inbox.as_view()),
+    re_path(r"%s/inbox/?$" % LOCAL_USER_PATH, views.Inbox.as_view()),
-    re_path(r"%s/outbox/?$" % local_user_path, views.Outbox.as_view()),
+    re_path(r"%s/outbox/?$" % LOCAL_USER_PATH, views.Outbox.as_view()),
     re_path(r"^\.well-known/webfinger/?$", views.webfinger),
     re_path(r"^\.well-known/nodeinfo/?$", views.nodeinfo_pointer),
     re_path(r"^\.well-known/host-meta/?$", views.host_meta),
@@ -182,7 +182,7 @@ urlpatterns = [
         r"^direct-messages/?$", views.DirectMessage.as_view(), name="direct-messages"
     ),
     re_path(
-        r"^direct-messages/(?P<username>%s)?$" % regex.username,
+        r"^direct-messages/(?P<username>%s)?$" % regex.USERNAME,
         views.DirectMessage.as_view(),
         name="direct-messages-user",
     ),
@@ -192,21 +192,21 @@ urlpatterns = [
     re_path(r"^import/?$", views.Import.as_view(), name="import"),
     re_path(r"^import/(\d+)/?$", views.ImportStatus.as_view(), name="import-status"),
     # users
-    re_path(r"%s/?$" % user_path, views.User.as_view(), name="user-feed"),
+    re_path(r"%s/?$" % USER_PATH, views.User.as_view(), name="user-feed"),
-    re_path(r"%s\.json$" % user_path, views.User.as_view()),
+    re_path(r"%s\.json$" % USER_PATH, views.User.as_view()),
-    re_path(r"%s/rss" % user_path, views.rss_feed.RssFeed(), name="user-rss"),
+    re_path(r"%s/rss" % USER_PATH, views.rss_feed.RssFeed(), name="user-rss"),
     re_path(
-        r"%s/followers(.json)?/?$" % user_path,
+        r"%s/followers(.json)?/?$" % USER_PATH,
         views.Followers.as_view(),
         name="user-followers",
     ),
     re_path(
-        r"%s/following(.json)?/?$" % user_path,
+        r"%s/following(.json)?/?$" % USER_PATH,
         views.Following.as_view(),
         name="user-following",
     ),
     # lists
-    re_path(r"%s/lists/?$" % user_path, views.UserLists.as_view(), name="user-lists"),
+    re_path(r"%s/lists/?$" % USER_PATH, views.UserLists.as_view(), name="user-lists"),
     re_path(r"^list/?$", views.Lists.as_view(), name="lists"),
     re_path(r"^list/(?P<list_id>\d+)(.json)?/?$", views.List.as_view(), name="list"),
     re_path(r"^list/add-book/?$", views.list.add_book, name="list-add-book"),
@@ -224,14 +224,14 @@ urlpatterns = [
         r"^list/(?P<list_id>\d+)/curate/?$", views.Curate.as_view(), name="list-curate"
     ),
     # User books
-    re_path(r"%s/books/?$" % user_path, views.Shelf.as_view(), name="user-shelves"),
+    re_path(r"%s/books/?$" % USER_PATH, views.Shelf.as_view(), name="user-shelves"),
     re_path(
-        r"^%s/(helf|books)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % user_path,
+        r"^%s/(helf|books)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % USER_PATH,
         views.Shelf.as_view(),
         name="shelf",
     ),
     re_path(
-        r"^%s/(books|shelf)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % local_user_path,
+        r"^%s/(books|shelf)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % LOCAL_USER_PATH,
         views.Shelf.as_view(),
         name="shelf",
     ),
@@ -241,7 +241,7 @@ urlpatterns = [
     re_path(r"^unshelve/?$", views.unshelve),
     # goals
     re_path(
-        r"%s/goal/(?P<year>\d{4})/?$" % user_path,
+        r"%s/goal/(?P<year>\d{4})/?$" % USER_PATH,
         views.Goal.as_view(),
         name="user-goal",
     ),
@@ -258,10 +258,10 @@ urlpatterns = [
     re_path(r"^block/(?P<user_id>\d+)/?$", views.Block.as_view()),
     re_path(r"^unblock/(?P<user_id>\d+)/?$", views.unblock),
     # statuses
-    re_path(r"%s(.json)?/?$" % status_path, views.Status.as_view(), name="status"),
+    re_path(r"%s(.json)?/?$" % STATUS_PATH, views.Status.as_view(), name="status"),
-    re_path(r"%s/activity/?$" % status_path, views.Status.as_view(), name="status"),
+    re_path(r"%s/activity/?$" % STATUS_PATH, views.Status.as_view(), name="status"),
     re_path(
-        r"%s/replies(.json)?/?$" % status_path, views.Replies.as_view(), name="replies"
+        r"%s/replies(.json)?/?$" % STATUS_PATH, views.Replies.as_view(), name="replies"
     ),
     re_path(
         r"^post/?$",
@@ -289,17 +289,17 @@ urlpatterns = [
     re_path(r"^boost/(?P<status_id>\d+)/?$", views.Boost.as_view()),
     re_path(r"^unboost/(?P<status_id>\d+)/?$", views.Unboost.as_view()),
     # books
-    re_path(r"%s(.json)?/?$" % book_path, views.Book.as_view(), name="book"),
+    re_path(r"%s(.json)?/?$" % BOOK_PATH, views.Book.as_view(), name="book"),
     re_path(
-        r"%s/(?P<user_statuses>review|comment|quote)/?$" % book_path,
+        r"%s/(?P<user_statuses>review|comment|quote)/?$" % BOOK_PATH,
         views.Book.as_view(),
         name="book-user-statuses",
     ),
-    re_path(r"%s/edit/?$" % book_path, views.EditBook.as_view()),
+    re_path(r"%s/edit/?$" % BOOK_PATH, views.EditBook.as_view()),
-    re_path(r"%s/confirm/?$" % book_path, views.ConfirmEditBook.as_view()),
+    re_path(r"%s/confirm/?$" % BOOK_PATH, views.ConfirmEditBook.as_view()),
     re_path(r"^create-book/?$", views.EditBook.as_view(), name="create-book"),
     re_path(r"^create-book/confirm?$", views.ConfirmEditBook.as_view()),
-    re_path(r"%s/editions(.json)?/?$" % book_path, views.Editions.as_view()),
+    re_path(r"%s/editions(.json)?/?$" % BOOK_PATH, views.Editions.as_view()),
     re_path(
         r"^upload-cover/(?P<book_id>\d+)/?$", views.upload_cover, name="upload-cover"
     ),

bookwyrm/utils/__init__.py

@@ -1 +1,2 @@
-from .regex import username
+""" useful regex """
+from .regex import USERNAME

bookwyrm/utils/regex.py

@@ -1,10 +1,10 @@
 """ defining regexes for regularly used concepts """
 
-domain = r"[\w_\-\.]+\.[a-z]{2,}"
+DOMAIN = r"[\w_\-\.]+\.[a-z]{2,}"
-localname = r"@?[a-zA-Z_\-\.0-9]+"
+LOCALNAME = r"@?[a-zA-Z_\-\.0-9]+"
-strict_localname = r"@[a-zA-Z_\-\.0-9]+"
+STRICT_LOCALNAME = r"@[a-zA-Z_\-\.0-9]+"
-username = r"%s(@%s)?" % (localname, domain)
+USERNAME = r"%s(@%s)?" % (LOCALNAME, DOMAIN)
-strict_username = r"\B%s(@%s)?\b" % (strict_localname, domain)
+STRICT_USERNAME = r"\B%s(@%s)?\b" % (STRICT_LOCALNAME, DOMAIN)
-full_username = r"%s@%s\b" % (localname, domain)
+FULL_USERNAME = r"%s@%s\b" % (LOCALNAME, DOMAIN)
 # should match (BookWyrm/1.0.0; or (BookWyrm/99.1.2;
-bookwyrm_user_agent = r"\(BookWyrm/[0-9]+\.[0-9]+\.[0-9]+;"
+BOOKWYRM_USER_AGENT = r"\(BookWyrm/[0-9]+\.[0-9]+\.[0-9]+;"

bookwyrm/views/helpers.py

@@ -38,7 +38,7 @@ def is_api_request(request):
 def is_bookwyrm_request(request):
     """check if the request is coming from another bookwyrm instance"""
     user_agent = request.headers.get("User-Agent")
-    if user_agent is None or re.search(regex.bookwyrm_user_agent, user_agent) is None:
+    if user_agent is None or re.search(regex.BOOKWYRM_USER_AGENT, user_agent) is None:
         return False
     return True
 

bookwyrm/views/inbox.py

@@ -21,6 +21,7 @@ from bookwyrm.utils import regex
 class Inbox(View):
     """requests sent by outside servers"""
 
+    # pylint: disable=too-many-return-statements
     def post(self, request, username=None):
         """only works as POST request"""
         # first check if this server is on our shitlist
@@ -70,7 +71,7 @@ def is_blocked_user_agent(request):
     user_agent = request.headers.get("User-Agent")
     if not user_agent:
         return False
-    url = re.search(r"https?://{:s}/?".format(regex.domain), user_agent)
+    url = re.search(r"https?://{:s}/?".format(regex.DOMAIN), user_agent)
     if not url:
         return False
     url = url.group()

bookwyrm/views/isbn.py

@@ -1,13 +1,8 @@
 """ isbn search view """
-from django.http import HttpResponseNotFound
 from django.http import JsonResponse
-from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
-from django.utils.decorators import method_decorator
 from django.views import View
-from django.views.decorators.http import require_POST
 
-from bookwyrm import forms, models
 from bookwyrm.connectors import connector_manager
 from .helpers import is_api_request
 
@@ -23,7 +18,6 @@ class Isbn(View):
             return JsonResponse([r.json() for r in book_results], safe=False)
 
         data = {
-            "title": "ISBN Search Results",
             "results": book_results,
             "query": isbn,
         }

bookwyrm/views/list.py

@@ -314,8 +314,7 @@ def set_book_position(request, list_item_id):
             Max("order")
         )["order__max"]
 
-        if int_position > order_max:
+        int_position = min(int_position, order_max)
-            int_position = order_max
 
         if request.user not in (book_list.user, list_item.user):
             return HttpResponseNotFound()

bookwyrm/views/rss_feed.py

@@ -10,7 +10,7 @@ class RssFeed(Feed):
     description_template = "rss/content.html"
     title_template = "rss/title.html"
 
-    def get_object(self, request, username):
+    def get_object(self, request, username):  # pylint: disable=arguments-differ
         """the user who's posts get serialized"""
         return get_user_from_username(request.user, username)
 

bookwyrm/views/search.py

@@ -83,7 +83,7 @@ def user_search(query, viewer, *_):
 
     # use webfinger for mastodon style account@domain.com username to load the user if
     # they don't exist locally (handle_remote_webfinger will check the db)
-    if re.match(regex.full_username, query):
+    if re.match(regex.FULL_USERNAME, query):
         handle_remote_webfinger(query)
 
     return (

bookwyrm/views/shelf.py

@@ -17,7 +17,7 @@ from bookwyrm import forms, models
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.settings import PAGE_LENGTH
 from .helpers import is_api_request, get_edition, get_user_from_username
-from .helpers import handle_reading_status, privacy_filter
+from .helpers import privacy_filter
 
 
 # pylint: disable=no-self-use

bookwyrm/views/status.py

@@ -133,7 +133,7 @@ def find_mentions(content):
     """detect @mentions in raw status content"""
     if not content:
         return
-    for match in re.finditer(regex.strict_username, content):
+    for match in re.finditer(regex.STRICT_USERNAME, content):
         username = match.group().strip().split("@")[1:]
         if len(username) == 1:
             # this looks like a local user (@user), fill in the domain
@@ -150,7 +150,7 @@ def find_mentions(content):
 def format_links(content):
     """detect and format links"""
     return re.sub(
-        r'([^(href=")]|^|\()(https?:\/\/(%s([\w\.\-_\/+&\?=:;,])*))' % regex.domain,
+        r'([^(href=")]|^|\()(https?:\/\/(%s([\w\.\-_\/+&\?=:;,])*))' % regex.DOMAIN,
         r'\g<1><a href="\g<2>">\g<3></a>',
         content,
     )