Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Commit

Permalink
Add and enable mypy type hinting
Browse files Browse the repository at this point in the history
Plus fix or ignore the various breaches that have been highlighted. I'm
ignoring these for now as a few of them are fairly gnarly/non-trivial,
but people should feel free to pick at them in the future if they feel
inclined to do so.
  • Loading branch information
samuelhwilliams committed May 14, 2024
1 parent 2c24a72 commit f40d4e9
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ repos:
hooks:
- id: djlint-jinja
types_or: ['html', 'jinja']
- repo: https://github.com/pre-commit/mirrors-mypy
rev: 'v1.10.0' # Use the sha / tag you want to point at
hooks:
- id: mypy
additional_dependencies: [types-jmespath, types-requests, types-beautifulsoup4, types-flask]
6 changes: 3 additions & 3 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def create_app(config_class=Config) -> Flask:
logging.init_app(app)
app.jinja_env.lstrip_blocks = True
app.jinja_env.trim_blocks = True
app.jinja_loader = ChoiceLoader(
app.jinja_loader = ChoiceLoader( # type: ignore # TODO: fixme
[
PackageLoader("app"),
PrefixLoader(
Expand Down Expand Up @@ -111,8 +111,8 @@ def setup_funds_and_auth(app: Flask) -> None:

app.config["AUTH_MAPPINGS"] = AuthService(
fund_to_auth_mappings={
towns_fund.fund_name: AuthMapping(towns_fund.auth_class, tf_auth),
pathfinders.fund_name: AuthMapping(pathfinders.auth_class, pf_auth),
towns_fund.fund_name: AuthMapping(towns_fund.auth_class, tf_auth), # type: ignore # TODO: fixme
pathfinders.fund_name: AuthMapping(pathfinders.auth_class, pf_auth), # type: ignore # TODO: fixme
}
)

Expand Down
2 changes: 1 addition & 1 deletion app/main/data_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from config import Config


def post_ingest(file: FileStorage, data: dict = None) -> tuple[dict | None, dict | None, dict | None]:
def post_ingest(file: FileStorage, data: dict | None = None) -> tuple[dict | None, dict | None, dict | None]:
"""Send an HTTP POST request to ingest into the data store
server and return the response.
Expand Down
3 changes: 1 addition & 2 deletions app/main/notify.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from datetime import datetime
from typing import BinaryIO

import notifications_python_client as notify
from flask import current_app
Expand All @@ -16,7 +15,7 @@
def send_email(
email_address: str,
template_id: str,
file: BinaryIO | FileStorage = None,
file: FileStorage | None = None,
**kwargs,
) -> None:
"""Send email to the specified email address via the GovUK Notify service.
Expand Down
4 changes: 3 additions & 1 deletion app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from config import Config


def get_friendly_fund_type(fund_type_id: str) -> str | None:
def get_friendly_fund_type(fund_type_id) -> str | None:
"""Gets a friendly fund type name from an ID.
If fund_type_id is not recognised, it is logged and None is returned.
Expand All @@ -19,6 +19,8 @@ def get_friendly_fund_type(fund_type_id: str) -> str | None:
except KeyError:
current_app.logger.error("Unknown fund type id found: {fund_type_id}", extra=dict(fund_type_id=fund_type_id))

return None


def days_between_dates(date1: date, date2: date) -> int:
"""Calculate the number of days between two dates.
Expand Down
10 changes: 5 additions & 5 deletions config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
case "unit_test":
from config.envs.unit_test import UnitTestConfig as Config
case "development":
from config.envs.development import DevelopmentConfig as Config
from config.envs.development import DevelopmentConfig as Config # type: ignore # TODO: fixme
case "dev" | "test" | "production":
from config.envs.aws import AwsConfig as Config
from config.envs.aws import AwsConfig as Config # type: ignore # TODO: fixme
case _:
from config.envs.default import DefaultConfig as Config
from config.envs.default import DefaultConfig as Config # type: ignore # TODO: fixme

try:
Config.pretty_print()
Config.pretty_print() # type: ignore # TODO: fixme
except AttributeError:
print({"msg": "Config doesn't have pretty_print function."})

__all__ = [Config]
__all__ = ["Config"]
2 changes: 1 addition & 1 deletion config/envs/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class DefaultConfig(object):
# RSA 256 KEYS
RSA256_PUBLIC_KEY_BASE64 = os.getenv("RSA256_PUBLIC_KEY_BASE64")
if RSA256_PUBLIC_KEY_BASE64:
RSA256_PUBLIC_KEY = base64.b64decode(RSA256_PUBLIC_KEY_BASE64).decode()
RSA256_PUBLIC_KEY: str = base64.b64decode(RSA256_PUBLIC_KEY_BASE64).decode()

TF_ADDITIONAL_EMAIL_LOOKUPS = ast.literal_eval(os.getenv("TF_ADDITIONAL_EMAIL_LOOKUPS", "{}"))
if not isinstance(TF_ADDITIONAL_EMAIL_LOOKUPS, dict):
Expand Down
2 changes: 1 addition & 1 deletion config/envs/development.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class DevelopmentConfig(DefaultConfig):
# RSA 256 KEYS
if not hasattr(DefaultConfig, "RSA256_PUBLIC_KEY"):
_test_public_key_path = DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
with open(_test_public_key_path, mode="rb") as public_key_file:
with open(_test_public_key_path, mode="r") as public_key_file:
RSA256_PUBLIC_KEY = public_key_file.read()

# devs can submit for these LAs and places
Expand Down
2 changes: 1 addition & 1 deletion config/envs/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class UnitTestConfig(DefaultConfig):
# RSA 256 KEYS
if not hasattr(DefaultConfig, "RSA256_PUBLIC_KEY"):
_test_public_key_path = DefaultConfig.FLASK_ROOT + "/tests/keys/rsa256/public.pem"
with open(_test_public_key_path, mode="rb") as public_key_file:
with open(_test_public_key_path, mode="r") as public_key_file:
RSA256_PUBLIC_KEY = public_key_file.read()

EXAMPLE_INGEST_WRONG_FORMAT = DefaultConfig.FLASK_ROOT + "/tests/resources/wrong_format_test_file.txt"
Expand Down
File renamed without changes.
11 changes: 11 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,14 @@ exclude = [
"cache",
]
mccabe.max-complexity = 12

[tool.mypy]
python_version = "3.11"

[[tool.mypy.overrides]]
module = [
"fsd_utils.*",
"notifications_python_client.*",
"flask_assets"
]
ignore_missing_imports = true
1 change: 1 addition & 0 deletions requirements-dev.in
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pip-tools==6.14.0
ruff
pep8-naming==0.13.3
pur==7.1.0
mypy

#-----------------------------------
# Testing
Expand Down
5 changes: 5 additions & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ markupsafe==2.1.3
# werkzeug
mccabe==0.7.0
# via flake8
mypy==1.10.0
# via -r requirements-dev.in
mypy-extensions==1.0.0
# via mypy
notifications-python-client==8.1.0
# via -r requirements.txt
packaging==23.1
Expand Down Expand Up @@ -288,6 +292,7 @@ typing-extensions==4.7.1
# via
# -r requirements.txt
# alembic
# mypy
# sqlalchemy
urllib3==1.26.18
# via
Expand Down
4 changes: 0 additions & 4 deletions static_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,3 @@ def build_bundles(static_folder):
bundles = init_assets(static_folder=static_folder)
for bundle in bundles:
bundle.build()


if __name__ == "__main__":
build_bundles()
14 changes: 8 additions & 6 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Generator

import pytest
from flask.testing import FlaskClient
from werkzeug.datastructures import FileStorage
Expand Down Expand Up @@ -48,7 +50,7 @@ def mocked_pf_and_tf_auth(mocker):


@pytest.fixture()
def flask_test_client(mocked_auth) -> FlaskClient:
def flask_test_client(mocked_auth) -> Generator[FlaskClient, None, None]:
"""
Creates the test client we will be using to test the responses
from our app, this is a test fixture.
Expand All @@ -62,7 +64,7 @@ def flask_test_client(mocked_auth) -> FlaskClient:


@pytest.fixture()
def unauthenticated_flask_test_client() -> FlaskClient:
def unauthenticated_flask_test_client() -> Generator[FlaskClient, None, None]:
"""
:return: An unauthenticated flask test client.
"""
Expand All @@ -71,14 +73,14 @@ def unauthenticated_flask_test_client() -> FlaskClient:


@pytest.fixture(scope="function")
def example_pre_ingest_data_file() -> FileStorage:
def example_pre_ingest_data_file() -> Generator[FileStorage, None, None]:
"""An example pre ingest spreadsheet in towns-fund format."""
with open(UnitTestConfig.EXAMPLE_INGEST_DATA_PATH, "rb") as file:
yield file
yield FileStorage(file)


@pytest.fixture(scope="function")
def example_ingest_wrong_format() -> FileStorage:
def example_ingest_wrong_format() -> Generator[FileStorage, None, None]:
"""An example pre ingest spreadsheet in towns-fund format."""
with open(UnitTestConfig.EXAMPLE_INGEST_WRONG_FORMAT, "rb") as file:
yield file
yield FileStorage(file)

0 comments on commit f40d4e9

Please sign in to comment.