OpenConceptLab/ocl_issues#960 | fixed pylint warnings

Author: snyaggarwal

.pylintrc

@@ -1,5 +1,5 @@
 [MESSAGES CONTROL]
-disable=missing-docstring,too-few-public-methods,no-member,too-many-ancestors,attribute-defined-outside-init,fixme,broad-except,import-outside-toplevel,too-many-instance-attributes,unsupported-membership-test,too-many-public-methods,too-many-lines
+disable=missing-docstring,too-few-public-methods,no-member,too-many-ancestors,attribute-defined-outside-init,fixme,broad-except,import-outside-toplevel,too-many-instance-attributes,unsupported-membership-test,too-many-public-methods,too-many-lines,unused-private-member,consider-using-with,duplicate-code
 [MASTER]
 ignore=migrations,fixtures,scripts,commands,documents.py,password_validation.py,v1_dump
 [FORMAT]

core/client_configs/models.py

@@ -82,7 +82,7 @@ def validate_home_tabs_config(self):
             self.errors = dict(tabs=[EMPTY_TABS_CONFIG])
         elif not isinstance(tabs, list):
             self.errors = dict(tabs=[NOT_LIST_TABS_CONFIG])
-        elif any([not isinstance(tab, dict) for tab in tabs]):
+        elif any(not isinstance(tab, dict) for tab in tabs):
             self.errors = dict(tabs=[INVALID_TABS_CONFIG])
         else:
             default_tabs = [tab for tab in tabs if tab.get('default', False)]
@@ -105,7 +105,7 @@ def validate_sort_attributes(self):
                 return
 
     def __get_resource_sortable_fields(self, tab_type):
-        es_fields = self.__get_es_fields(tab_type) or dict()
+        es_fields = self.__get_es_fields(tab_type) or {}
         return [field for field, config in es_fields.items() if config.get('sortable', False)]
 
     @staticmethod

core/client_configs/tests.py

@@ -17,7 +17,7 @@ def test_is_home(self):
         self.assertFalse(ClientConfig(type='blah').is_home)
 
     def test_home_config_validation(self):
-        client_config = ClientConfig(config=dict())
+        client_config = ClientConfig(config={})
 
         with self.assertRaises(ValidationError) as ex:
             client_config.full_clean()
@@ -153,7 +153,7 @@ def tearDown(self):
     def test_post(self):
         response = self.client.post(
             self.org.url + 'client-configs/',
-            dict(),
+            {},
             HTTP_AUTHORIZATION='Token ' + self.token,
             format='json'
         )

core/collections/models.py

@@ -159,7 +159,7 @@ def check_concept_uniqueness_in_collection_and_locale_by_name_attribute(
         if not other_concepts_in_collection.exists():
             return
 
-        matching_names_in_concept = dict()
+        matching_names_in_concept = {}
         names = concept.names.filter(**{attribute: value})
 
         for name in names:
@@ -229,7 +229,7 @@ def add_references_in_bulk(self, expressions, user=None):  # pylint: disable=too
                 new_expressions.discard(existing_expression)
                 errors[existing_expression] = [REFERENCE_ALREADY_EXISTS]
 
-        added_references = list()
+        added_references = []
         for expression in new_expressions:
             ref = CollectionReference(expression=expression)
             try:

core/collections/serializers.py

@@ -226,7 +226,7 @@ class Meta:
     def __init__(self, *args, **kwargs):
         params = get(kwargs, 'context.request.query_params')
 
-        self.query_params = dict()
+        self.query_params = {}
         if params:
             self.query_params = params if isinstance(params, dict) else params.dict()
         self.include_summary = self.query_params.get(INCLUDE_SUMMARY) in ['true', True]

core/collections/views.py

@@ -9,7 +9,7 @@
 from pydash import get
 from rest_framework import status, mixins
 from rest_framework.generics import (
-    RetrieveAPIView, DestroyAPIView, UpdateAPIView, ListAPIView, RetrieveUpdateDestroyAPIView
+    RetrieveAPIView, DestroyAPIView, UpdateAPIView, ListAPIView
 )
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
@@ -31,12 +31,12 @@
     CollectionVersionSummaryDetailSerializer, CollectionReferenceDetailSerializer)
 from core.collections.utils import is_version_specified
 from core.common.constants import (
-    HEAD, RELEASED_PARAM, PROCESSING_PARAM, OK_MESSAGE, NOT_FOUND, MUST_SPECIFY_EXTRA_PARAM_IN_BODY
+    HEAD, RELEASED_PARAM, PROCESSING_PARAM, OK_MESSAGE
 )
 from core.common.mixins import (
     ConceptDictionaryCreateMixin, ListWithHeadersMixin, ConceptDictionaryUpdateMixin,
     ConceptContainerExportMixin,
-    ConceptContainerProcessingMixin)
+    ConceptContainerProcessingMixin, ConceptContainerExtraRetrieveUpdateDestroyView)
 from core.common.permissions import (
     CanViewConceptDictionary, CanEditConceptDictionary, HasAccessToVersionedObject,
     CanViewConceptDictionaryVersion
@@ -535,51 +535,10 @@ def list(self, request, *args, **kwargs):
         return Response(get(self.get_object(), 'extras', {}))
 
 
-class CollectionExtraRetrieveUpdateDestroyView(CollectionExtrasBaseView, RetrieveUpdateDestroyAPIView):
+class CollectionExtraRetrieveUpdateDestroyView(CollectionExtrasBaseView,
+                                               ConceptContainerExtraRetrieveUpdateDestroyView):
     serializer_class = CollectionDetailSerializer
 
-    def retrieve(self, request, *args, **kwargs):
-        key = kwargs.get('extra')
-        instance = self.get_object()
-        extras = get(instance, 'extras', {})
-        if key in extras:
-            return Response({key: extras[key]})
-
-        return Response(dict(detail=NOT_FOUND), status=status.HTTP_404_NOT_FOUND)
-
-    def update(self, request, **kwargs):  # pylint: disable=arguments-differ
-        key = kwargs.get('extra')
-        value = request.data.get(key)
-        if not value:
-            return Response([MUST_SPECIFY_EXTRA_PARAM_IN_BODY.format(key)], status=status.HTTP_400_BAD_REQUEST)
-
-        instance = self.get_object()
-        instance.extras = get(instance, 'extras', {})
-        instance.extras[key] = value
-        instance.comment = 'Updated extras: %s=%s.' % (key, value)
-        head = instance.get_head()
-        head.extras = get(head, 'extras', {})
-        head.extras.update(instance.extras)
-        instance.save()
-        head.save()
-        return Response({key: value})
-
-    def delete(self, request, *args, **kwargs):
-        key = kwargs.get('extra')
-        instance = self.get_object()
-        instance.extras = get(instance, 'extras', {})
-        if key in instance.extras:
-            del instance.extras[key]
-            instance.comment = 'Deleted extra %s.' % key
-            head = instance.get_head()
-            head.extras = get(head, 'extras', {})
-            del head.extras[key]
-            instance.save()
-            head.save()
-            return Response(status=status.HTTP_204_NO_CONTENT)
-
-        return Response(dict(detail=NOT_FOUND), status=status.HTTP_404_NOT_FOUND)
-
 
 class CollectionVersionProcessingView(CollectionBaseView, ConceptContainerProcessingMixin):
     serializer_class = CollectionVersionDetailSerializer

core/common/healthcheck/healthcheck.py

@@ -27,7 +27,7 @@ def check_status(self):
             if not response.ok:
                 raise ServiceUnavailable('Flower Unavailable')
         except Exception as ex:
-            raise ServiceUnavailable(ex.args)
+            raise ServiceUnavailable(ex.args) from ex
 
     def identifier(self):
         return "Flower"
@@ -45,7 +45,7 @@ def check_status(self):
             if not is_ok:
                 raise ServiceReturnedUnexpectedResult("Status {}".format(status))
         except Exception as ex:
-            raise ServiceReturnedUnexpectedResult(ex.args)
+            raise ServiceReturnedUnexpectedResult(ex.args) from ex
 
     def identifier(self):
         return 'ElasticSearch'

core/common/mixins.py

@@ -11,11 +11,13 @@
 from django.utils.functional import cached_property
 from pydash import compact, get
 from rest_framework import status
+from rest_framework.generics import RetrieveUpdateDestroyAPIView
 from rest_framework.mixins import ListModelMixin, CreateModelMixin
 from rest_framework.response import Response
 
 from core.common.constants import HEAD, ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW, ACCESS_TYPE_NONE, INCLUDE_FACETS, \
-    LIST_DEFAULT_LIMIT, HTTP_COMPRESS_HEADER, CSV_DEFAULT_LIMIT, FACETS_ONLY
+    LIST_DEFAULT_LIMIT, HTTP_COMPRESS_HEADER, CSV_DEFAULT_LIMIT, FACETS_ONLY, NOT_FOUND, \
+    MUST_SPECIFY_EXTRA_PARAM_IN_BODY
 from core.common.permissions import HasPrivateAccess, HasOwnership, CanViewConceptDictionary
 from core.common.services import S3
 from .utils import write_csv_to_s3, get_csv_from_s3, get_query_params_from_url_string, compact_dict_by_values
@@ -128,7 +130,7 @@ def list(self, request, *args, **kwargs):  # pylint:disable=too-many-locals
 
         sorted_list = self.object_list
 
-        headers = dict()
+        headers = {}
         results = sorted_list
         if not compress:
             paginator = CustomPaginator(
@@ -460,7 +462,7 @@ def from_uri_queryset(cls, uri):
         queryset = cls.objects.none()
 
         try:
-            kwargs = get(resolve(uri), 'kwargs', dict())
+            kwargs = get(resolve(uri), 'kwargs', {})
             query_params = get_query_params_from_url_string(uri)  # parsing query parameters
             kwargs.update(query_params)
             if 'concept' in kwargs:
@@ -484,7 +486,7 @@ def global_listing_queryset(cls, params, user):
 
     @staticmethod
     def get_parent_and_owner_filters_from_uri(uri):
-        filters = dict()
+        filters = {}
         if not uri:
             return filters
 
@@ -639,3 +641,47 @@ def post(self, request, *args, **kwargs):  # pylint: disable=unused-argument
         version.clear_processing()
 
         return Response(status=status.HTTP_200_OK)
+
+
+class ConceptContainerExtraRetrieveUpdateDestroyView(RetrieveUpdateDestroyAPIView):
+    def retrieve(self, request, *args, **kwargs):
+        key = kwargs.get('extra')
+        instance = self.get_object()
+        extras = get(instance, 'extras', {})
+        if key in extras:
+            return Response({key: extras[key]})
+
+        return Response(dict(detail=NOT_FOUND), status=status.HTTP_404_NOT_FOUND)
+
+    def update(self, request, **kwargs):  # pylint: disable=arguments-differ
+        key = kwargs.get('extra')
+        value = request.data.get(key)
+        if not value:
+            return Response([MUST_SPECIFY_EXTRA_PARAM_IN_BODY.format(key)], status=status.HTTP_400_BAD_REQUEST)
+
+        instance = self.get_object()
+        instance.extras = get(instance, 'extras', {})
+        instance.extras[key] = value
+        instance.comment = 'Updated extras: %s=%s.' % (key, value)
+        head = instance.get_head()
+        head.extras = get(head, 'extras', {})
+        head.extras.update(instance.extras)
+        instance.save()
+        head.save()
+        return Response({key: value})
+
+    def delete(self, request, *args, **kwargs):
+        key = kwargs.get('extra')
+        instance = self.get_object()
+        instance.extras = get(instance, 'extras', {})
+        if key in instance.extras:
+            del instance.extras[key]
+            instance.comment = 'Deleted extra %s.' % key
+            head = instance.get_head()
+            head.extras = get(head, 'extras', {})
+            del head.extras[key]
+            instance.save()
+            head.save()
+            return Response(status=status.HTTP_204_NO_CONTENT)
+
+        return Response(dict(detail=NOT_FOUND), status=status.HTTP_404_NOT_FOUND)

core/common/models.py

@@ -300,7 +300,7 @@ def versioned_object_url(self):
     @classmethod
     def get_version(cls, mnemonic, version=HEAD, filters=None):
         if not filters:
-            filters = dict()
+            filters = {}
         return cls.objects.filter(**{cls.mnemonic_attr: mnemonic, **filters}, version=version).first()
 
     def get_latest_version(self):
@@ -502,7 +502,7 @@ def seed_references():
 
     @classmethod
     def persist_new(cls, obj, created_by, **kwargs):
-        errors = dict()
+        errors = {}
         parent_resource = kwargs.pop('parent_resource', None) or obj.parent
         if not parent_resource:
             errors['parent'] = PARENT_RESOURCE_CANNOT_BE_NONE
@@ -541,7 +541,7 @@ def persist_new_version(cls, obj, user=None, **kwargs):
         from core.collections.serializers import CollectionDetailSerializer
         from core.sources.serializers import SourceDetailSerializer
 
-        errors = dict()
+        errors = {}
 
         obj.is_active = True
         if user:
@@ -566,7 +566,7 @@ def persist_new_version(cls, obj, user=None, **kwargs):
 
     @classmethod
     def persist_changes(cls, obj, updated_by, original_schema, **kwargs):
-        errors = dict()
+        errors = {}
         parent_resource = kwargs.pop('parent_resource', obj.parent)
         if not parent_resource:
             errors['parent'] = SOURCE_PARENT_CANNOT_BE_NONE
@@ -708,7 +708,7 @@ def is_processing(self):
         return False
 
     def clear_processing(self):
-        self._background_process_ids = list()
+        self._background_process_ids = []
         self.save(update_fields=['_background_process_ids'])
 
     @property

core/common/services.py

@@ -49,6 +49,8 @@ def generate_signed_url(cls, accessor, key, metadata=None):
         except NoCredentialsError:  # pragma: no cover
             pass
 
+        return None
+
     @classmethod
     def upload(cls, file_path, file_content, headers=None, metadata=None):
         url = cls.generate_signed_url(cls.PUT, file_path, metadata)
@@ -67,7 +69,7 @@ def upload_file(
     ):  # pylint: disable=too-many-arguments
         read_directive = 'rb' if binary else 'r'
         file_path = file_path if file_path else key
-        return cls.upload(key, open(file_path, read_directive).read(), headers, metadata)
+        return cls.upload(key, open(file_path, read_directive, encoding='utf-8').read(), headers, metadata)
 
     @classmethod
     def upload_public(cls, file_path, file_content):
@@ -82,6 +84,8 @@ def upload_public(cls, file_path, file_content):
         except NoCredentialsError:  # pragma: no cover
             pass
 
+        return None
+
     @classmethod
     def upload_base64(  # pylint: disable=too-many-arguments,inconsistent-return-statements
             cls, doc_base64, file_name, append_extension=True, public_read=False, headers=None
@@ -173,7 +177,7 @@ def missing_objects(cls, objects, prefix_path, sub_paths):  # pragma: no cover
 
         for obj in objects:
             paths = [obj.pdf_path(path) for path in sub_paths]
-            if not all([path in s3_keys for path in paths]):
+            if not all(path in s3_keys for path in paths):
                 missing_objects.append(obj)
 
         return missing_objects
@@ -189,6 +193,8 @@ def remove(cls, key):
         except NoCredentialsError:  # pragma: no cover
             pass
 
+        return None
+
 
 class RedisService:  # pragma: no cover
     def __init__(self):