Merge pull request #309 from concept-to-clinic/lamby/wip/code-style

Improve various code styles

docs/conf.py

@@ -20,6 +20,7 @@
 import os
 import sys
 import django
+
 from recommonmark.transform import AutoStructify
 
 DOCS_DIR = os.getcwd()
@@ -73,8 +74,12 @@
 # directories to ignore when looking for source files.
 # This patterns also effect to html_static_path and html_extra_path
 exclude_patterns = [
-    '_build', 'Thumbs.db', '.DS_Store', 'apidoc_interface/modules.rst',
-    'apidoc_prediction/modules.rst']
+    '_build',
+    'Thumbs.db',
+    '.DS_Store',
+    'apidoc_interface/modules.rst',
+    'apidoc_prediction/modules.rst',
+]
 
 # The name of the Pygments (syntax highlighting) style to use.
 pygments_style = 'sphinx'

interface/backend/api/serializers.py

@@ -15,11 +15,13 @@
 
 
 class ImageFileSerializer(serializers.ModelSerializer):
-    """ Serializes an ImageFile model object.
+    """
+    Serializes an ImageFile model object.
 
-        In addition to the properties on the object, it returns a `preview_url` which
-        will return the dicom image data encoded in base64.
+    In addition to the properties on the object, it returns a `preview_url`
+    which will return the dicom image data encoded in base64.
     """
+
     class Meta:
         model = ImageFile
         fields = '__all__'
@@ -31,11 +33,13 @@ def get_preview_url(self, instance):
 
 
 class ImageSeriesSerializer(serializers.HyperlinkedModelSerializer):
-    """ Serializes an ImageSeries model object.
+    """
+    Serializes an ImageSeries model object.
 
-        Will also contain an `images` property with all of the serialized
-        image files in this series.
+    Will also contain an `images` property with all of the serialized image
+    files in this series.
     """
+
     class Meta:
         model = ImageSeries
         fields = '__all__'
@@ -97,12 +101,13 @@ def create(self, validated_data):
 
 
 class CaseSerializer(serializers.HyperlinkedModelSerializer):
-    """ Serializes a Case model object
+    """
+    Serializes a Case model object
 
-        Case is the top level of the object hierarchy, and it contains
-        the image series, candidates, and nodules for the currently active
-        case.
+    Case is the top level of the object hierarchy, and it contains the
+    image series, candidates, and nodules for the currently active case.
     """
+
     class Meta:
         model = Case
         fields = '__all__'

interface/backend/api/tests.py

@@ -115,10 +115,12 @@ def test_candidates_dismiss(self):
 
 
 class SerializerTest(TestCase):
-    """ Tests the serializers that perform any custom logic.
+    """
+    Tests the serializers that perform any custom logic.
 
-        In general, create logic is tested by posting in the ViewTest test cases.
+    In general, create logic is tested by posting in the ViewTest test cases.
     """
+
     def test_image_file_serializer(self):
         image_file = ImageFile(path='/my_path')
         serialized = ImageFileSerializer(image_file)

interface/backend/api/urls.py

@@ -1,4 +1,4 @@
-from collections import OrderedDict
+import collections
 
 from backend.api.views import (
     CaseViewSet,
@@ -21,11 +21,13 @@
 
 
 class RelativeUrlRootView(routers.APIRootView):
-    """ Provides relative URLs for the available endpoints.
     """
+    Provides relative URLs for the available endpoints.
+    """
+
     def get(self, request, *args, **kwargs):
         # Return a plain {"name": "hyperlink"} response.
-        ret = OrderedDict()
+        ret = collections.OrderedDict()
         namespace = request.resolver_match.namespace
         for key, url_name in self.api_root_dict.items():
             if namespace:

interface/backend/images/factories.py

@@ -7,11 +7,11 @@ class ImageSeriesFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = models.ImageSeries
 
-    patient_id = factory.Sequence(lambda n: "TEST-SERIES-%04d" % n)
+    patient_id = factory.Sequence(lambda n: "TEST-SERIES-{:4d}".format(n))
 
-    series_instance_uid = factory.Sequence(lambda n: "1.3.6.1.4.1.14519.5.2.1.6279.6001.%030d" % n)
+    series_instance_uid = factory.Sequence(lambda n: "1.3.6.1.4.1.14519.5.2.1.6279.6001.{:30d}".format(n))
 
-    uri = factory.LazyAttribute(lambda f: 'file:///tmp/%s/' % f.series_instance_uid)
+    uri = factory.LazyAttribute(lambda f: 'file:///tmp/{}/'.format(f.series_instance_uid))
 
 
 class ImageLocationFactory(factory.django.DjangoModelFactory):

interface/backend/images/models.py

@@ -12,9 +12,11 @@
 
 
 class ImageFile(models.Model):
-    """ Model for an individual file for an image--that is, a single DICOM slice
-        as it is represented on disk.
     """
+    Model for an individual file for an image--that is, a single DICOM slice as
+    it is represented on disk.
+    """
+
     # Explicitly map all the dicom properties that we need to parse and their
     # model equivalents; create a lambda that returns a dict for any type conversions
     # or one-to-many relationships
@@ -35,42 +37,47 @@ class ImageFile(models.Model):
     columns = models.IntegerField(null=True)
     pixel_spacing_col = models.FloatField(null=True)
     pixel_spacing_row = models.FloatField(null=True)
-    path = models.FilePathField(path=settings.DATASOURCE_DIR,
-                                recursive=True,
-                                allow_files=True,
-                                allow_folders=False,
-                                max_length=512)
+    path = models.FilePathField(
+        path=settings.DATASOURCE_DIR,
+        recursive=True,
+        allow_files=True,
+        allow_folders=False,
+        max_length=512,
+    )
 
     class Meta:
-        ordering = ['slice_location']
+        ordering = ('slice_location',)
 
     def save(self, *args, **kwargs):
         self._populate_dicom_properties()
+
         super().save(*args, **kwargs)
 
     def _populate_dicom_properties(self):
-        """ Parse DICOM properties from the file and store the relevant ones
-            on the object.
         """
+        Parse DICOM properties from the file and store the relevant ones on the
+        object.
+        """
+
         logger = logging.getLogger(__name__)
         for k, v in self.load_dicom_data_from_disk(self.path)['metadata'].items():
             # check that the field is a property on the object
-            if not hasattr(self.__class__, k):
-                logger.warning(f"Property '{k}' not a field on ImageFile, discarding value '{v}'.")
-
-            else:
+            if hasattr(self.__class__, k):
                 self.__setattr__(k, v)
+            else:
+                logger.warning(f"Property '{k}' not a field on ImageFile, discarding value '{v}'.")
 
     @classmethod
     def load_dicom_data_from_disk(cls, filepath, parse_metadata=True, encode_image_data=False):
-        """ Loads the dicom properties that we care about from disk.
+        """
+        Loads the dicom properties that we care about from disk.
 
-            For the values we load, see ImageFile.DICOM_PROPERTIES
+        For the values we load, see ImageFile.DICOM_PROPERTIES
 
-            Args:
-                filepath (str): the path to the file (so this can be used if ImageFile is not in database yet)
-                parse_metadata (bool, optional): weather or not to parse any metadata from the file (skip for just img)
-                encode_image_data (bool, optional): Base64 encode image data and return it.
+        Args:
+            filepath (str): the path to the file (so this can be used if ImageFile is not in database yet)
+            parse_metadata (bool, optional): weather or not to parse any metadata from the file (skip for just img)
+            encode_image_data (bool, optional): Base64 encode image data and return it.
         """
         if not os.path.abspath(filepath).startswith(settings.DATASOURCE_DIR):
             raise PermissionDenied
@@ -87,8 +94,9 @@ def load_dicom_data_from_disk(cls, filepath, parse_metadata=True, encode_image_d
 
     @classmethod
     def _parse_metadata(cls, dcm_data, filepath):
-        """ Parses metadata from dicom file and creates a dictionary
-            which we can use to populate the ImageFile object
+        """
+        Parses metadata from dicom file and creates a dictionary which we can
+        use to populate the ImageFile object
         """
         logger = logging.getLogger(__name__)
 
@@ -101,7 +109,7 @@ def _update_once(d1, d2):
                 else:
                     d1[k] = v
 
-        metadata = dict()
+        metadata = {}
         for dicom_prop, model_attribute in cls.DICOM_PROPERTIES.items():
             if callable(model_attribute):
                 try:
@@ -117,7 +125,9 @@ def _update_once(d1, d2):
         return metadata
 
     def get_image_data(self):
-        """ Shortcut for just the encoded image from `load_dicom_properties_from_disk`.
+        """
+        Shortcut for just the encoded image from
+        `load_dicom_properties_from_disk`.
         """
         return self.load_dicom_data_from_disk(self.path, parse_metadata=False, encode_image_data=True)['image']
 
@@ -133,6 +143,7 @@ def _dicom_to_base64(cls, ds):
         Returning base64 encoded string for a dicom image
         """
         rescaled = cls._pixel_data2str(ds.pixel_array)
+
         return base64.b64encode(rescaled.tobytes())
 
 
@@ -147,33 +158,43 @@ class ImageSeries(models.Model):
     @classmethod
     def get_or_create(cls, uri):
         """
-        Return the ImageSeries instance with the same PatientID and SeriesInstanceUID as the DICOM images in the
-        given directory. If none exists so far, create one.
-        Return a tuple of (ImageSeries, created), where created is a boolean specifying whether the object was created.
+        Return the ImageSeries instance with the same PatientID and
+        SeriesInstanceUID as the DICOM images in the given directory. If none
+        exists so far, create one.
+
+        Returns a tuple of (ImageSeries, created), where created is a boolean
+        specifying whether the object was created.
 
         Args:
-            uri (str): absolute URI to a directory with DICOM images of a patient
+            uri (str): absolute URI to a directory with DICOM images of a
+                       patient
 
         Returns:
-            (ImageSeries, bool): the looked up ImageSeries instance and whether it had to be created
+            (ImageSeries, bool): the looked up ImageSeries instance and whether
+                                 it had to be created
         """
-        # get all the images in the folder that are valid dicom extensions
-        files = [f for f in os.listdir(uri)
-                 if os.path.splitext(f)[-1] in settings.IMAGE_EXTENSIONS]
 
-        # load series-level metadata from the first dicom file
+        # Get all the images in the folder that are valid dicom extensions
+        files = [
+            f for f in os.listdir(uri)
+            if os.path.splitext(f)[-1] in settings.IMAGE_EXTENSIONS
+        ]
+
+        # Load series-level metadata from the first dicom file
         plan = dicom.read_file(safe_join(uri, files[0]))
 
         patient_id = plan.PatientID
         series_instance_uid = plan.SeriesInstanceUID
 
-        series, created = ImageSeries.objects.get_or_create(patient_id=patient_id,
-                                                            series_instance_uid=series_instance_uid,
-                                                            uri=uri)
+        series, created = ImageSeries.objects.get_or_create(
+            patient_id=patient_id,
+            series_instance_uid=series_instance_uid,
+            uri=uri,
+        )
 
-        # create models that point to each of the individual image files
-        for f in files:
-            image, _ = ImageFile.objects.get_or_create(path=safe_join(uri, f), series=series)
+        # Create models that point to each of the individual image files
+        for x in files:
+            image, _ = ImageFile.objects.get_or_create(path=safe_join(uri, x), series=series)
 
         return series, created
 
@@ -183,8 +204,9 @@ def __str__(self):
 
 class ImageLocation(models.Model):
     """
-    Model representing a certain voxel location on certain image
+    Model representing a certain voxel location on certain image.
     """
+
     x = models.PositiveSmallIntegerField(help_text='Voxel index for X axis, zero-index, from top left')
     y = models.PositiveSmallIntegerField(help_text='Voxel index for Y axis, zero-index, from top left')
     z = models.PositiveSmallIntegerField(help_text='Slice index for Z axis, zero-index')

interface/config/settings/base.py

@@ -9,6 +9,7 @@
 For the full list of settings and their values, see
 https://docs.djangoproject.com/en/1.11/ref/settings/
 """
+
 import environ
 
 BASE_DIR = environ.Path(__file__) - 3

interface/config/settings/test.py

@@ -55,6 +55,7 @@
 # ------------------------------------------------------------------------------
 # Keep templates in memory so tests run faster
 TEMPLATES[0].pop('APP_DIRS', None)
+
 TEMPLATES[0]['OPTIONS']['loaders'] = [
     ['django.template.loaders.cached.Loader', [
         'django.template.loaders.filesystem.Loader',

prediction/config.py

@@ -1,31 +1,29 @@
 """
-    prediction.config
-    ~~~~~~~~~~~~~~~~~
-
-    Provides the flask config options
+Provides the flask config options
 """
+
 import os
 
-from os import path
+from os.path import abspath, dirname, join, realpath
 
-LIDC_WILDCARD = ['LIDC-IDRI-*', '**', '**']
+LIDC_WILDCARD = ('LIDC-IDRI-*', '**', '**')
 
 
 class Config(object):
     PROD_SERVER = os.getenv('PRODUCTION', False)
     DEBUG = False
-    CURRENT_DIR = path.dirname(path.realpath(__file__))
-    PARENT_DIR = path.dirname(CURRENT_DIR)
-    ALGOS_DIR = path.abspath(path.join(CURRENT_DIR, 'src', 'algorithms'))
-    ALGO_CLASSIFY_GTR123_PATH = os.path.join(ALGOS_DIR, 'classify', 'assets', 'gtr123_model.ckpt')
-    SEGMENT_ASSETS_DIR = path.abspath(path.join(ALGOS_DIR, 'segment', 'assets'))
-    ALGO_SEGMENT_PATH = os.path.join(SEGMENT_ASSETS_DIR, 'best_model.hdf5')
-    FULL_DICOM_PATHS = path.join(PARENT_DIR, 'images_full')
-    SMALL_DICOM_PATHS = path.join(PARENT_DIR, 'images')
-    FULL_DICOM_PATHS_WILDCARD = path.join(FULL_DICOM_PATHS, *LIDC_WILDCARD)
-    SMALL_DICOM_PATHS_WILDCARD = path.join(FULL_DICOM_PATHS, *LIDC_WILDCARD)
-    DATA_DIR = path.abspath(path.join(CURRENT_DIR, 'data'))
-    EXTRACTED_IMAGE_DIR = path.abspath(path.join(CURRENT_DIR, 'extracted'))
+    CURRENT_DIR = dirname(realpath(__file__))
+    PARENT_DIR = dirname(CURRENT_DIR)
+    ALGOS_DIR = abspath(join(CURRENT_DIR, 'src', 'algorithms'))
+    ALGO_CLASSIFY_GTR123_PATH = join(ALGOS_DIR, 'classify', 'assets', 'gtr123_model.ckpt')
+    SEGMENT_ASSETS_DIR = abspath(join(ALGOS_DIR, 'segment', 'assets'))
+    ALGO_SEGMENT_PATH = join(SEGMENT_ASSETS_DIR, 'best_model.hdf5')
+    FULL_DICOM_PATHS = join(PARENT_DIR, 'images_full')
+    SMALL_DICOM_PATHS = join(PARENT_DIR, 'images')
+    FULL_DICOM_PATHS_WILDCARD = join(FULL_DICOM_PATHS, *LIDC_WILDCARD)
+    SMALL_DICOM_PATHS_WILDCARD = join(FULL_DICOM_PATHS, *LIDC_WILDCARD)
+    DATA_DIR = abspath(join(CURRENT_DIR, 'data'))
+    EXTRACTED_IMAGE_DIR = abspath(join(CURRENT_DIR, 'extracted'))
 
 
 class Production(Config):