Merge pull request #3038 from SEED-platform/refactor/init-type-hints

Refactor/init type hints

.github/workflows/ci.yml

@@ -105,14 +105,14 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        tox_env: [flake8, docs]
+        tox_env: [flake8, docs, mypy]
     steps:
       -
         uses: actions/checkout@v2
       -
         uses: actions/setup-python@v2
         with:
-          python-version: '3.7'
+          python-version: '3.9'
       -
         name: Install deps
         run: |

config/settings/common.py

@@ -84,7 +84,7 @@
 
 ROOT_URLCONF = 'config.urls'
 
-INSTALLED_APPS = (
+DJANGO_CORE_APPS = (
     'django.contrib.auth',
     'django.contrib.contenttypes',
     'django.contrib.flatpages',
@@ -131,7 +131,7 @@
 # Internal apps can resolve this via South's depends_on.
 HIGH_DEPENDENCY_APPS = ('seed.landing',)  # 'landing' contains SEEDUser
 
-INSTALLED_APPS = HIGH_DEPENDENCY_APPS + INSTALLED_APPS + SEED_CORE_APPS
+INSTALLED_APPS = HIGH_DEPENDENCY_APPS + DJANGO_CORE_APPS + SEED_CORE_APPS
 
 # apps to auto load name spaced URLs for JS use (see seed.urls)
 SEED_URL_APPS = (

docs/source/developer_resources.rst

@@ -31,6 +31,44 @@ To run flake locally call:
 
     tox -e flake8
 
+Python Type Hints
+^^^^^^^^^^^^^^^^^
+
+Python type hints are beginning to be added to the SEED codebase. The benefits are
+eliminating some accidental typing mistakes to prevent bugs as well as a better IDE
+experience.
+
+Usage
+*****
+
+SEED does not require exhaustive type annotations, but it is recommended you add them if you
+create any new functions or refactor any existing code where it might be beneficial
+and not require a ton of additional effort.
+
+When applicable, we recommend you use `built-in collection types <https://docs.python.org/3/whatsnew/3.9.html#type-hinting-generics-in-standard-collections>`_
+such as :code:`list`, :code:`dict` or :code:`tuple` instead of the capitalized types
+from the :code:`typing` module.
+
+Common gotchas:
+- If trying to annotate a class method with the class itself, import :code:`from __future__ import annotations`
+- If you're getting warnings about runtime errors due to a type name, make sure your IDE is set up to point to an environment with python 3.9
+- If you're wasting time trying to please the type checker, feel free to throw :code:`# type: ignore` on the problematic line (or at the top of the file to ignore all issues for that file)
+
+Type Checking
+*************
+
+CI currently runs static type checking on the codebase using `mypy <http://mypy-lang.org/>`_. For
+your own IDE, we recommend the following extensions:
+
+- VSCode: `Pylance <https://marketplace.visualstudio.com/items?itemName=ms-python.vscode-pylance>`_ (uses Microsoft's Pyright type checking)
+
+To run the same typechecking applied in CI (i.e. using mypy) you can run the following
+
+.. code-block:: console
+
+    tox -e mypy
+
+
 Django Notes
 ------------
 

mypy.ini

@@ -0,0 +1,5 @@
+[mypy]
+ignore_missing_imports = True
+
+[mypy-seed.*.migrations.*]
+ignore_errors = True

requirements/test.txt

@@ -31,3 +31,6 @@ hypothesis==6.12.0
 
 # For running the server
 uWSGI==2.0.18
+
+# static type checking
+mypy==0.910

seed/data_importer/__init__.py

@@ -4,9 +4,3 @@
 :copyright (c) 2014 - 2021, The Regents of the University of California, through Lawrence Berkeley National Laboratory (subject to receipt of any required approvals from the U.S. Department of Energy) and contributors. All rights reserved.  # NOQA
 :author
 """
-# monkey-patch to suppress threading error message in python 2.7.3
-# See http://stackoverflow.com/questions/13193278/understand-python-threading-bug
-import sys
-if sys.version_info[:3] == (2, 7, 3):
-    import threading
-    threading._DummyThread._Thread__stop = lambda x: 42

seed/data_importer/models.py

@@ -11,10 +11,7 @@
 import math
 import tempfile
 
-try:
-    from urllib import unquote  # python2.x
-except ImportError:
-    from urllib.parse import unquote  # python3.x
+from urllib.parse import unquote
 
 from django.contrib.auth.models import User
 from django.urls import reverse
@@ -537,11 +534,6 @@ def SUMMARY_ANALYSIS_ACTIVE_KEY(cls, pk):
     def SUMMARY_ANALYSIS_QUEUED_KEY(cls, pk):
         return 'SUMMARY_ANALYSIS_QUEUED%s' % pk
 
-    @property
-    def form(self, data=None):
-        from seed.data_importer import ImportRecordForm
-        return ImportRecordForm(data, instance=self)
-
     def prefixed_pk(self, pk, max_len_before_prefix=(SOURCE_FACILITY_ID_MAX_LEN - len('IMP1234-'))):
         """This is a total hack to support prefixing until source_facility_id
         is turned into a proper pk.  Prefixes a given pk with the import_record"""

seed/models/derived_columns.py

@@ -4,7 +4,9 @@
 :copyright (c) 2014 - 2021, The Regents of the University of California, through Lawrence Berkeley National Laboratory (subject to receipt of any required approvals from the U.S. Department of Energy) and contributors. All rights reserved.  # NOQA
 :author
 """
-from copy import copy
+from __future__ import annotations
+import copy
+from typing import Any, Union
 
 from django.db import models
 from django.core.exceptions import ValidationError
@@ -20,7 +22,7 @@
 from seed.models.tax_lots import TaxLotState
 
 
-def _cast_params_to_floats(params):
+def _cast_params_to_floats(params: dict[str, Any]) -> dict[str, float]:
     """Helper to turn dict values to floats or remove them if non-numeric
 
     :param params: dict{str: <value>}
@@ -113,7 +115,7 @@ def set_params(self, params):
             """
             self.params = params
 
-    def __init__(self, expression, validate=True):
+    def __init__(self, expression: str, validate: bool = True):
         """Construct an expression evaluator.
 
         :param expression: str
@@ -127,7 +129,7 @@ def __init__(self, expression, validate=True):
         self._parser = Lark(self.EXPRESSION_GRAMMAR, parser='lalr', transformer=self._transformer)
 
     @classmethod
-    def is_valid(cls, expression):
+    def is_valid(cls, expression: str) -> bool:
         """Validate the expression. Raises an InvalidExpression exception if invalid
 
         :param expression: str
@@ -140,7 +142,7 @@ def is_valid(cls, expression):
 
         return True
 
-    def evaluate(self, parameters=None):
+    def evaluate(self, parameters: Union[None, dict[str, float]] = None) -> float:
         """Evaluate the expression with the provided parameters
 
         :param parameters: dict, keys are parameter names and values are values
@@ -150,7 +152,7 @@ def evaluate(self, parameters=None):
             parameters = {}
 
         self._transformer.set_params(parameters)
-        return self._parser.parse(self._expression)
+        return self._parser.parse(self._expression)  # type: ignore[return-value]
 
 
 class InvalidExpression(Exception):
@@ -223,7 +225,7 @@ def save(self, *args, **kwargs):
         self.full_clean()
         return super().save(*args, **kwargs)
 
-    def get_parameter_values(self, inventory_state):
+    def get_parameter_values(self, inventory_state: Union[PropertyState, TaxLotState]) -> dict[str, Any]:
         """Construct a dictionary of column values keyed by expression parameter
         names. Note that no cleaning / validation is done to the values, they are
         straight from the database, or if a column is not found for the inventory
@@ -239,8 +241,6 @@ def get_parameter_values(self, inventory_state):
                 ...
             }
         """
-        assert isinstance(inventory_state, self.INVENTORY_TYPE_TO_CLASS[self.inventory_type])
-
         if not hasattr(self, '_cached_column_parameters'):
             self._cached_column_parameters = (
                 DerivedColumnParameter.objects
@@ -262,7 +262,7 @@ def get_parameter_values(self, inventory_state):
 
         return params
 
-    def evaluate(self, inventory_state=None, parameters=None):
+    def evaluate(self, inventory_state: Union[None, PropertyState, TaxLotState] = None, parameters: Union[None, dict[str, float]] = None):
         """Evaluate the expression. Caller must provide `parameters`, `inventory_state`,
         or both. Values from the inventory take priority over the parameters dict.
         Values that cannot be coerced into floats (from the inventory or params dict)
@@ -289,7 +289,7 @@ def evaluate(self, inventory_state=None, parameters=None):
             tmp_params = self.get_parameter_values(inventory_state)
             inventory_parameters = _cast_params_to_floats(tmp_params)
 
-        merged_parameters = copy(parameters)
+        merged_parameters = copy.copy(parameters)
         merged_parameters.update(inventory_parameters)
         merged_parameters = _cast_params_to_floats(merged_parameters)
 

seed/models/meters.py

@@ -64,7 +64,7 @@ class Meter(models.Model):
         (ELECTRICITY_UNKNOWN, 'Electric - Unknown'),
     )
 
-    type_lookup = dict((reversed(type) for type in ENERGY_TYPES))
+    type_lookup = dict((reversed(type) for type in ENERGY_TYPES))  # type: ignore
 
     PORTFOLIO_MANAGER = 1
     GREENBUTTON = 2

seed/serializers/properties.py

@@ -339,7 +339,7 @@ class PropertyViewAsStateSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = PropertyView
-        validators = []
+        validators = []  # type: ignore[var-annotated]
         fields = ('id', 'state', 'property', 'cycle',
                   'changed_fields', 'date_edited',
                   'certifications', 'filename', 'history',

seed/tests/api/test_seed_host_api.py

@@ -88,10 +88,10 @@
 
 # API is now used basic auth with base64 encoding.
 # NOTE: The header only accepts lower case usernames.
-auth_string = base64.urlsafe_b64encode(bytes(
+encoded_credentials = base64.urlsafe_b64encode(bytes(
     '{}:{}'.format(username.lower(), api_key), 'utf-8'
 ))
-auth_string = 'Basic {}'.format(auth_string.decode('utf-8'))
+auth_string = 'Basic {}'.format(encoded_credentials.decode('utf-8'))
 header = {
     'Authorization': auth_string,
     # "Content-Type": "application/json"

seed/tests/test_analysis_pipelines.py

@@ -618,7 +618,7 @@ def _mock_request(file_, model_type):
 
     # Skipping this test b/c of an unexpected error validating BuildingSync files
     # See here for more info: https://github.com/SEED-platform/seed/pull/2901
-    @skip
+    @skip('See https://github.com/SEED-platform/seed/pull/2901')
     def test_build_bsyncr_input_returns_valid_bsync_document(self):
         # Act
         doc, errors = _build_bsyncr_input(self.analysis_property_view, self.meter)

seed/tests/util.py

@@ -129,7 +129,7 @@ class FakeRequest(object):
     META = {'REMOTE_ADDR': '127.0.0.1'}
     path = 'fake_login_path'
     body = None
-    GET = POST = {}
+    GET = POST = {}  # type: ignore
 
     def __init__(self, data=None, headers=None, user=None, method='POST', **kwargs):
         if 'body' in kwargs:

seed/utils/api_schema.py

@@ -6,8 +6,6 @@
 
 
 class AutoSchemaHelper(SwaggerAutoSchema):
-    # overrides the serialization of existing endpoints
-    overwrite_params = []
 
     # Used to easily build out example values displayed on Swagger page.
     openapi_types = {
@@ -178,14 +176,6 @@ def schema_factory(cls, obj, **kwargs):
 
         raise Exception(f'Unhandled type "{type(obj)}" for {obj}')
 
-    def add_manual_parameters(self, parameters):
-        manual_params = self.manual_fields.get((self.method, self.view.action), [])
-
-        if (self.method, self.view.action) in self.overwrite_params:
-            return manual_params
-        # I think this should add to existing parameters, but haven't been able to confirm.
-        return parameters + manual_params
-
 
 # this is a commonly used swagger decorator so moved here for DRYness
 swagger_auto_schema_org_query_param = swagger_auto_schema(

seed/utils/viewsets.py

@@ -14,6 +14,7 @@
 """
 
 # Imports from Django
+from typing import Any
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.parsers import FormParser, JSONParser, MultiPartParser
 from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet
@@ -56,7 +57,7 @@ def perform_update(self, serializer):
         return UpdateModelMixin.perform_update(self, serializer)
 
 
-class SEEDOrgModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetMixin, ModelViewSet):
+class SEEDOrgModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetMixin, ModelViewSet):  # type: ignore[misc]
     """Viewset class customized with SEED standard attributes.
 
     Attributes:
@@ -66,12 +67,12 @@ class SEEDOrgModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetMixin, Mo
             SessionAuthentication and SEEDAuthentication.
     """
     renderer_classes = RENDERER_CLASSES
-    parser_classes = PARSER_CLASSES
+    parser_classes: 'tuple[Any, ...]' = PARSER_CLASSES
     authentication_classes = AUTHENTICATION_CLASSES
     permission_classes = PERMISSIONS_CLASSES
 
 
-class SEEDOrgReadOnlyModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetMixin,
+class SEEDOrgReadOnlyModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetMixin,  # type: ignore[misc]
                                   ReadOnlyModelViewSet):
     """Viewset class customized with SEED standard attributes.
 
@@ -82,7 +83,7 @@ class SEEDOrgReadOnlyModelViewSet(DecoratorMixin(drf_api_endpoint), OrgQuerySetM
             SessionAuthentication and SEEDAuthentication.
     """
     renderer_classes = RENDERER_CLASSES
-    parser_classes = PARSER_CLASSES
+    parser_classes: 'tuple[Any, ...]' = PARSER_CLASSES
     authentication_classes = AUTHENTICATION_CLASSES
     permission_classes = PERMISSIONS_CLASSES
 

seed/validators.py

@@ -5,7 +5,7 @@
 
 
 class PasswordBaseCharacterQuantityValidator(object):
-    TYPE = None
+    TYPE = ''
     RE = re.compile(r'')
 
     def __init__(self, quantity=1):

seed/views/labels.py

@@ -37,7 +37,7 @@
 ErrorState = namedtuple('ErrorState', ['status_code', 'message'])
 
 
-class LabelViewSet(DecoratorMixin(drf_api_endpoint), viewsets.ModelViewSet):
+class LabelViewSet(DecoratorMixin(drf_api_endpoint), viewsets.ModelViewSet):  # type: ignore[misc]
     """API endpoint for viewing and creating labels.
 
             Returns::

seed/views/portfoliomanager.py

@@ -19,10 +19,7 @@
 from rest_framework.decorators import action
 from rest_framework.viewsets import GenericViewSet
 
-try:
-    from urllib import quote  # python2.x
-except ImportError:
-    from urllib.parse import quote  # python3.x
+from urllib.parse import quote
 
 _log = logging.getLogger(__name__)
 

seed/views/projects.py

@@ -64,7 +64,7 @@
 PLURALS = {'property': 'properties', 'taxlot': 'taxlots'}
 
 
-class ProjectViewSet(DecoratorMixin(drf_api_endpoint), viewsets.ModelViewSet):
+class ProjectViewSet(DecoratorMixin(drf_api_endpoint), viewsets.ModelViewSet):  # type: ignore[misc]
     serializer_class = ProjectSerializer
     renderer_classes = (JSONRenderer,)
     parser_classes = (JSONParser,)

seed/views/reports.py

@@ -35,7 +35,7 @@
 from xlsxwriter import Workbook
 
 
-class Report(DecoratorMixin(drf_api_endpoint), ViewSet):
+class Report(DecoratorMixin(drf_api_endpoint), ViewSet):  # type: ignore[misc]
     renderer_classes = (JSONRenderer,)
     parser_classes = (JSONParser,)
 

seed/views/v3/labels.py

@@ -81,7 +81,7 @@
         )
     ),
 )
-class LabelViewSet(DecoratorMixin(drf_api_endpoint), SEEDOrgNoPatchOrOrgCreateModelViewSet):
+class LabelViewSet(DecoratorMixin(drf_api_endpoint), SEEDOrgNoPatchOrOrgCreateModelViewSet):  # type: ignore[misc]
     """
     retrieve:
         Return a label instance by pk if it is within user`s specified org.