From 0cc42d4f926667e0acf2dfb4a9b8dbf70d2a59e6 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 14 May 2026 13:04:26 +0100 Subject: [PATCH 1/4] Ruff changes, general linting and formatting --- CHANGELOG.md | 4 +++ docs/api.md | 6 ++-- docs/number.md | 8 ++--- ruff.toml | 9 +++++ tasks/format.sh | 3 +- tests/test_api.py | 28 ++++++++------- tests/test_component.py | 8 ++--- tests/test_flask_talisman.py | 1 + tests/test_number.py | 20 +++++------ tests/test_security.py | 3 +- tests/test_url.py | 3 +- tna_utilities/__init__.py | 6 ++-- tna_utilities/api.py | 54 +++++++++++++++-------------- tna_utilities/component.py | 18 +++++----- tna_utilities/currency.py | 17 ++++------ tna_utilities/datetime.py | 60 ++++++++++++++++++--------------- tna_utilities/flask/talisman.py | 36 ++++++++++---------- tna_utilities/number.py | 19 +++++------ tna_utilities/security.py | 9 ++--- tna_utilities/url.py | 18 ++++------ 20 files changed, 170 insertions(+), 160 deletions(-) create mode 100644 ruff.toml diff --git a/CHANGELOG.md b/CHANGELOG.md index aa5babb..6836861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Updated Google CSP domains - Switched to checking response codes from `requests` in `SimpleJsonApiClient` +- Renamed `ResourceForbidden`, `ResourceNotFound` and `ResourceUnauthorized` to `ResourceForbiddenError`, `ResourceNotFoundError` and `ResourceUnauthorizedError` in `tna_utilities.api` +- `SimpleJsonApiClient` raises `ApiError` exceptions rather than generic `Exception` +- Renamed the `bytes` parameter of `pretty_file_size()` to `filesize_bytes` ### Deprecated @@ -27,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed logic inversion for `default_headers` in `SimpleJsonApiClient` +- Incorrect `ValueError` exceptions changed to `TypeError` ### Security diff --git a/docs/api.md b/docs/api.md index c539910..6f4ef5c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -33,9 +33,9 @@ except Exception as error: You can catch and handle some of the more common exceptions: -- `tna_utilities.api.ResourceForbidden` -- `tna_utilities.api.ResourceNotFound` -- `tna_utilities.api.ResourceUnauthorized` +- `tna_utilities.api.ResourceForbiddenError` +- `tna_utilities.api.ResourceNotFoundError` +- `tna_utilities.api.ResourceUnauthorizedError` You can also catch [exceptions raised by `requests`](https://requests.readthedocs.io/en/latest/_modules/requests/exceptions/). diff --git a/docs/number.md b/docs/number.md index a303b8e..4138645 100644 --- a/docs/number.md +++ b/docs/number.md @@ -39,10 +39,10 @@ Formats file size to a human-readable string. ### Arguments -| Argument | Description | Default | -| ---------- | ---------------------------------------------------------- | ------- | -| `bytes` | The number of bytes | [none] | -| `simplify` | If `True`, simplify the string and remove fractional sizes | `True` | +| Argument | Description | Default | +| ---------------- | ---------------------------------------------------------- | ------- | +| `filesize_bytes` | The number of bytes | [none] | +| `simplify` | If `True`, simplify the string and remove fractional sizes | `True` | ### Example diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..9732550 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,9 @@ +extend = "/home/app/ruff-strict.toml" + +[lint] +ignore = [ + "TRY003", + "PLR0913", + "PLR0911", + "PLR0912" +] diff --git a/tasks/format.sh b/tasks/format.sh index dbbedf0..e07833b 100755 --- a/tasks/format.sh +++ b/tasks/format.sh @@ -1,3 +1,4 @@ #!/bin/bash -docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview format +# TODO: Change ruff version back to preview once the Docker image has been released +docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff format diff --git a/tests/test_api.py b/tests/test_api.py index 33ef823..9ff3542 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -2,10 +2,12 @@ from unittest import TestCase, mock from requests import Timeout + from tna_utilities.api import ( - ResourceForbidden, - ResourceNotFound, - ResourceUnauthorized, + ApiError, + ResourceForbiddenError, + ResourceNotFoundError, + ResourceUnauthorizedError, SimpleJsonApiClient, ) @@ -29,15 +31,15 @@ def json(self): if args[0] == f"{MOCK_API_BASE_URL}happy": return MockResponse({"foo": "bar"}, 200) - elif args[0] == f"{MOCK_API_BASE_URL}badrequest": + if args[0] == f"{MOCK_API_BASE_URL}badrequest": return MockResponse(None, 400) - elif args[0] == f"{MOCK_API_BASE_URL}unauthorized": + if args[0] == f"{MOCK_API_BASE_URL}unauthorized": return MockResponse(None, 401) - elif args[0] == f"{MOCK_API_BASE_URL}forbidden": + if args[0] == f"{MOCK_API_BASE_URL}forbidden": return MockResponse(None, 403) - elif args[0] == f"{MOCK_API_BASE_URL}servererror": + if args[0] == f"{MOCK_API_BASE_URL}servererror": return MockResponse(None, 500) - elif args[0] == f"{MOCK_API_BASE_URL}timeout": + if args[0] == f"{MOCK_API_BASE_URL}timeout": raise Timeout("Request timed out") return MockResponse(None, 404) @@ -80,28 +82,28 @@ def test_happy(self, mock_post, mock_get): @mock.patch("requests.post", side_effect=mocked_requests_post) def test_bad_request(self, mock_post, mock_get): client = SimpleJsonApiClient(MOCK_API_BASE_URL) - with self.assertRaises(Exception): + with self.assertRaises(ApiError): client.get("/badrequest") @mock.patch("requests.get", side_effect=mocked_requests_get) @mock.patch("requests.post", side_effect=mocked_requests_post) def test_not_found(self, mock_post, mock_get): client = SimpleJsonApiClient(MOCK_API_BASE_URL) - with self.assertRaises(ResourceNotFound): + with self.assertRaises(ResourceNotFoundError): client.get("/notfound") @mock.patch("requests.get", side_effect=mocked_requests_get) @mock.patch("requests.post", side_effect=mocked_requests_post) def test_resource_unauthorized(self, mock_post, mock_get): client = SimpleJsonApiClient(MOCK_API_BASE_URL) - with self.assertRaises(ResourceUnauthorized): + with self.assertRaises(ResourceUnauthorizedError): client.get("/unauthorized") @mock.patch("requests.get", side_effect=mocked_requests_get) @mock.patch("requests.post", side_effect=mocked_requests_post) def test_resource_forbidden(self, mock_post, mock_get): client = SimpleJsonApiClient(MOCK_API_BASE_URL) - with self.assertRaises(ResourceForbidden): + with self.assertRaises(ResourceForbiddenError): client.get("/forbidden") @mock.patch("requests.get", side_effect=mocked_requests_get) @@ -115,7 +117,7 @@ def test_resource_timeout(self, mock_post, mock_get): @mock.patch("requests.post", side_effect=mocked_requests_post) def test_other_exception(self, mock_post, mock_get): client = SimpleJsonApiClient(MOCK_API_BASE_URL) - with self.assertRaises(Exception): + with self.assertRaises(ApiError): client.get("/servererror") @mock.patch("requests.get", side_effect=mocked_requests_get) diff --git a/tests/test_component.py b/tests/test_component.py index 400d11d..2cc3095 100644 --- a/tests/test_component.py +++ b/tests/test_component.py @@ -309,19 +309,19 @@ def test_pagination_third_no_around(self): ) def test_pagination_no_pages(self): - with self.assertRaises(Exception): + with self.assertRaises(ValueError): paginate(0, 1) def test_pagination_invalid_pages(self): - with self.assertRaises(Exception): + with self.assertRaises(TypeError): paginate(None, 1) def test_pagination_negative_current_page(self): - with self.assertRaises(Exception): + with self.assertRaises(ValueError): paginate(42, -1) def test_pagination_negative_around(self): - with self.assertRaises(Exception): + with self.assertRaises(ValueError): paginate(42, 1, around=-1) def test_tna_pagination_items(self): diff --git a/tests/test_flask_talisman.py b/tests/test_flask_talisman.py index 5c91069..466e169 100644 --- a/tests/test_flask_talisman.py +++ b/tests/test_flask_talisman.py @@ -1,6 +1,7 @@ import unittest from flask import Flask, session + from tna_utilities.flask.talisman import Talisman diff --git a/tests/test_number.py b/tests/test_number.py index a7b5d50..eb185f7 100644 --- a/tests/test_number.py +++ b/tests/test_number.py @@ -4,7 +4,6 @@ class TestNumberish(unittest.TestCase): - def test_happy(self): self.assertEqual(numberish(0), "None") self.assertEqual(numberish(1), "1") @@ -86,18 +85,17 @@ def test_happy_prefix_tuple(self): ) def test_unhappy(self): - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): numberish("one") - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): numberish(None) - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): numberish({}) - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): numberish([]) class TestPrettyFileSize(unittest.TestCase): - def test_pretty_file_size(self): self.assertEqual(pretty_file_size(0), "0B") self.assertEqual(pretty_file_size(999), "999B") @@ -139,13 +137,13 @@ def test_pretty_file_size_unsimplified(self): self.assertEqual(pretty_file_size(1000000000000000, simplify=False), "1PB") def test_pretty_file_size_unhappy(self): - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): pretty_file_size(1.234) - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): pretty_file_size("one") - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): pretty_file_size(None) - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): pretty_file_size({}) - with self.assertRaises(ValueError): + with self.assertRaises(TypeError): pretty_file_size([]) diff --git a/tests/test_security.py b/tests/test_security.py index 834c7d6..affb9f9 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -4,9 +4,8 @@ class TestSecurityCSP(unittest.TestCase): - def __init__(self, *args, **kwargs): - super(TestSecurityCSP, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.test_domain = "https://example.com" self.test_domain_2 = "https://another.net" diff --git a/tests/test_url.py b/tests/test_url.py index 7c91c81..05936b8 100644 --- a/tests/test_url.py +++ b/tests/test_url.py @@ -4,9 +4,8 @@ class TestQueryStringTransformer(unittest.TestCase): - def __init__(self, *args, **kwargs): - super(TestQueryStringTransformer, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) self.test_query = [("a", ["1"]), ("b", ["2", "3"])] def test_init(self): diff --git a/tna_utilities/__init__.py b/tna_utilities/__init__.py index e0c0975..990d8cb 100644 --- a/tna_utilities/__init__.py +++ b/tna_utilities/__init__.py @@ -9,11 +9,11 @@ def strtobool(value: str) -> bool: """ try: value = value.lower() - except AttributeError: - raise TypeError("invalid truth value %r" % (value,)) + except AttributeError as e: + raise TypeError(f"invalid truth value {value!r}") from e if value in ("y", "yes", "t", "true", "on", "1"): return True if value in ("n", "no", "f", "false", "off", "0"): return False - raise ValueError("invalid truth value %r" % (value,)) + raise ValueError(f"invalid truth value {value!r}") diff --git a/tna_utilities/api.py b/tna_utilities/api.py index 91abd12..81923b6 100644 --- a/tna_utilities/api.py +++ b/tna_utilities/api.py @@ -2,15 +2,19 @@ from requests import JSONDecodeError, Response, codes -class ResourceNotFound(Exception): +class ApiError(Exception): pass -class ResourceForbidden(Exception): +class ResourceNotFoundError(ApiError): pass -class ResourceUnauthorized(Exception): +class ResourceForbiddenError(ApiError): + pass + + +class ResourceUnauthorizedError(ApiError): pass @@ -99,9 +103,9 @@ def get( :param params: Optional dictionary of query parameters to include in the request. These will be merged with any default parameters set for the client. :param headers: Optional dictionary of headers to include in the request. These will be merged with any default headers set for the client. :param timeout: Timeout in seconds for the request. Defaults to 10. - :raises ResourceNotFound: If the requested resource is not found (HTTP 404). - :raises ResourceForbidden: If access to the resource is forbidden (HTTP 403). - :raises ResourceUnauthorized: If authentication is required or has failed (HTTP 401). + :raises ResourceNotFoundError: If the requested resource is not found (HTTP 404). + :raises ResourceForbiddenError: If access to the resource is forbidden (HTTP 403). + :raises ResourceUnauthorizedError: If authentication is required or has failed (HTTP 401). :raises Exception: For unexpected response handling or request-processing errors. """ @@ -112,7 +116,7 @@ def get( headers=self.headers if headers is None else {**self.headers, **headers}, timeout=timeout, ) - return self._handle_response(response) + return self._handle_response(response, path) def post( self, @@ -133,9 +137,9 @@ def post( :param params: Optional dictionary of query parameters to include in the request. These will be merged with any default parameters set for the client. :param headers: Optional dictionary of headers to include in the request. These will be merged with any default headers set for the client. :param timeout: Request timeout in seconds. - :raises ResourceNotFound: If the requested resource is not found (HTTP 404). - :raises ResourceForbidden: If access to the resource is forbidden (HTTP 403). - :raises ResourceUnauthorized: If authentication is required or has failed (HTTP 401). + :raises ResourceNotFoundError: If the requested resource is not found (HTTP 404). + :raises ResourceForbiddenError: If access to the resource is forbidden (HTTP 403). + :raises ResourceUnauthorizedError: If authentication is required or has failed (HTTP 401). :raises Exception: For other non-success responses or unexpected response handling errors. """ @@ -148,9 +152,9 @@ def post( json=json, timeout=timeout, ) - return self._handle_response(response) + return self._handle_response(response, path) - def _handle_response(self, response: Response) -> dict: + def _handle_response(self, response: Response, path: str) -> dict: """ Handle the API response, checking for common HTTP status codes and returning the JSON content if the request was successful. """ @@ -158,27 +162,25 @@ def _handle_response(self, response: Response) -> dict: if response.status_code == codes.ok: try: return response.json() - except JSONDecodeError: - raise Exception("Non-JSON response provided") + except JSONDecodeError as e: + raise ApiError("Non-JSON response provided") from e if response.status_code == codes.bad_request: try: error_body = response.json() - except JSONDecodeError: + raise ApiError(f"Bad request: {error_body}") + except JSONDecodeError as e: error_body = response.text - raise Exception(f"Bad request: {error_body}") + raise ApiError(f"Bad request: {error_body}") from e if response.status_code == codes.unauthorized: - raise ResourceUnauthorized("Unauthorized") + raise ResourceUnauthorizedError("Unauthorized") if response.status_code == codes.forbidden: - raise ResourceForbidden("Forbidden") + raise ResourceForbiddenError("Forbidden") if response.status_code == codes.not_found: - raise ResourceNotFound("Resource not found") - body_preview = (response.text or "").strip() + raise ResourceNotFoundError("Resource not found") + body_preview = (response.text if hasattr(response, "text") else "").strip() if body_preview: body_preview = body_preview[:500] - raise Exception( - f"Request failed with status {response.status_code} for URL {response.url}. " - f"Response body: {body_preview}" + raise ApiError( + f"Request failed with status {response.status_code} for {path}. Response body: {body_preview}" ) - raise Exception( - f"Request failed with status {response.status_code} for URL {response.url}" - ) + raise ApiError(f"Request failed with status {response.status_code} for {path}") diff --git a/tna_utilities/component.py b/tna_utilities/component.py index be21c06..1206b6d 100644 --- a/tna_utilities/component.py +++ b/tna_utilities/component.py @@ -15,14 +15,14 @@ def paginate(pages: int, current_page: int, around: int = 1) -> list[int | str]: list: A list of dictionaries representing the paginated items, with "current" indicating the current page. """ - assert isinstance(pages, int), "pages must be an integer" - assert pages >= 1, "pages must be at least 1" - assert isinstance(current_page, int), "current_page must be an integer" - assert current_page >= 1, "current_page must be at least 1" - assert ( - current_page <= pages - ), "current_page cannot be greater than the number of pages" - assert around >= 0, "around must be non-negative" + if type(pages) is not int or type(current_page) is not int: + raise TypeError("pages and current_page must be integers") + if pages < 1 or current_page < 1: + raise ValueError("pages and current_page must be at least 1") + if current_page > pages: + raise ValueError("current_page cannot be greater than the number of pages") + if around < 0: + raise ValueError("around must be non-negative") items = [item + 1 for item in range(pages)] total = len(items) @@ -40,7 +40,7 @@ def paginate(pages: int, current_page: int, around: int = 1) -> list[int | str]: if around >= 1: for i in range(len(sorted_pages) - 1): - if sorted_pages[i + 1] - sorted_pages[i] == 2: + if sorted_pages[i + 1] - sorted_pages[i] == 2: # noqa: PLR2004 pagination.add(sorted_pages[i] + 1) sorted_pages = sorted(pagination) diff --git a/tna_utilities/currency.py b/tna_utilities/currency.py index 3d84408..0e10b8d 100644 --- a/tna_utilities/currency.py +++ b/tna_utilities/currency.py @@ -1,7 +1,4 @@ -from typing import Optional, Union - - -def currency(value: Union[float, str, int], simplify: bool = True) -> str: +def currency(value: float | str | int, simplify: bool = True) -> str: """ Formats a number as a currency without the currency symbol. @@ -14,13 +11,13 @@ def currency(value: Union[float, str, int], simplify: bool = True) -> str: int_number = int(float_number) if simplify and int_number == float_number: - return str("{:,}".format(int_number)) + return str(f"{int_number:,}") - return str("{:,.2f}".format(float_number)) + return str(f"{float_number:,.2f}") def pretty_price( - value: Union[float, str, int], + value: float | str | int, simplify: bool = True, currency_symbol: str = "£", ) -> str: @@ -30,15 +27,15 @@ def pretty_price( If the value is 0, returns "Free". Otherwise, returns the currency symbol followed by the formatted currency. """ - if value == 0 or value == "0" or round(float(value) * 100) == 0: + if value in {0, "0"} or round(float(value) * 100) == 0: return "Free" return f"{currency_symbol}{currency(value, simplify)}" def pretty_price_range( - value_from: Optional[Union[float, str, int]] = None, - value_to: Optional[Union[float, str, int]] = None, + value_from: float | str | int | None = None, + value_to: float | str | int | None = None, simplify: bool = True, currency_symbol: str = "£", ) -> str: diff --git a/tna_utilities/datetime.py b/tna_utilities/datetime.py index d7ddbb8..893267a 100644 --- a/tna_utilities/datetime.py +++ b/tna_utilities/datetime.py @@ -1,6 +1,5 @@ import datetime import math -from typing import Optional, Union """ See https://design-system.nationalarchives.gov.uk/content/dates-and-times/ @@ -9,7 +8,7 @@ """ -def get_date_from_string(date_string: str) -> datetime.datetime: # noqa: C901 +def get_date_from_string(date_string: str) -> datetime.datetime: """ Parses a date string into a datetime object. """ @@ -70,7 +69,7 @@ def _format_month_index(date: datetime.date) -> int: def pretty_date( - date: Union[str, datetime.date], + date: str | datetime.date, show_day: bool = False, ) -> str: """ @@ -116,7 +115,7 @@ def pretty_date( def pretty_datetime( - date: Union[str, datetime.datetime], + date: str | datetime.datetime, show_day: bool = False, show_seconds: bool = False, ) -> str: @@ -152,8 +151,8 @@ def pretty_datetime( def pretty_date_range( # noqa: C901 - date_from: Optional[Union[str, datetime.date]], - date_to: Optional[Union[str, datetime.date]], + date_from: str | datetime.date | None, + date_to: str | datetime.date | None, omit_days: bool = False, lowercase_first: bool = False, ) -> str: @@ -161,6 +160,9 @@ def pretty_date_range( # noqa: C901 Formats a date range into the format used by The National Archives. """ + max_days_in_month = 31 + max_months_in_year = 12 + if isinstance(date_from, datetime.date): pass elif isinstance(date_from, str): @@ -192,8 +194,8 @@ def pretty_date_range( # noqa: C901 if ( date_from.day == 1 and date_from.month == 1 - and date_to.day == 31 - and date_to.month == 12 + and date_to.day == max_days_in_month + and date_to.month == max_months_in_year ): if date_from.year == date_to.year: return str(date_from.year) @@ -237,8 +239,8 @@ def pretty_date_range( # noqa: C901 def pretty_datetime_range( # noqa: C901 - date_from: Optional[Union[str, datetime.date, datetime.datetime]], - date_to: Optional[Union[str, datetime.date, datetime.datetime]], + date_from: str | datetime.date | datetime.datetime | None, + date_to: str | datetime.date | datetime.datetime | None, lowercase_first: bool = False, hide_date_if_single_day: bool = False, show_seconds: bool = False, @@ -362,27 +364,31 @@ def pretty_age( prefix = "" suffix = " ago" - if days > 365: - years = days // 365 + days_in_year = 365 + days_in_month = 30 + seconds_in_hour = 3600 + seconds_in_minute = 60 + + if days > days_in_year: + years = days // days_in_year return f"{prefix}{years} year{'s' if years != 1 else ''}{suffix}" - elif days > 30: - months = days // 30 + if days > days_in_month: + months = days // days_in_month return f"{prefix}{months} month{'s' if months != 1 else ''}{suffix}" - elif days > 0: + if days > 0: return f"{prefix}{days} day{'s' if days != 1 else ''}{suffix}" - elif seconds >= 3600: - hours = seconds // 3600 + if seconds >= seconds_in_hour: + hours = seconds // seconds_in_hour return f"{prefix}{hours} hour{'s' if hours != 1 else ''}{suffix}" - elif seconds >= 60: - minutes = seconds // 60 + if seconds >= seconds_in_minute: + minutes = seconds // seconds_in_minute return f"{prefix}{minutes} minute{'s' if minutes != 1 else ''}{suffix}" - elif seconds > just_now_seconds or future: + if seconds > just_now_seconds or future: return f"{prefix}{seconds} second{'s' if seconds != 1 else ''}{suffix}" - else: - return "just now" if lowercase_first else "Just now" + return "just now" if lowercase_first else "Just now" -def is_today_or_future(date: Union[datetime.date, datetime.datetime]) -> bool: +def is_today_or_future(date: datetime.date | datetime.datetime) -> bool: """ Determines if the given date string represents today or a future date. """ @@ -398,8 +404,8 @@ def is_today_or_future(date: Union[datetime.date, datetime.datetime]) -> bool: def is_today_in_date_range( - date_from: Union[datetime.date, datetime.datetime], - date_to: Union[datetime.date, datetime.datetime], + date_from: datetime.date | datetime.datetime, + date_to: datetime.date | datetime.datetime, ) -> bool: """ Determines if today's date falls within the given date range. @@ -420,7 +426,7 @@ def is_today_in_date_range( def group_by_year_and_month( items: list[dict], date_key: str, reverse: bool = False -) -> list[dict]: # noqa: C901 +) -> list[dict]: """ Groups a list of items by year and month based on a date key in each item. """ @@ -582,7 +588,7 @@ def seconds_to_duration(total_seconds: int, simplify: bool = False) -> str: return " ".join(return_values) -def rfc_822_date_format(date: Union[datetime.date, datetime.datetime]) -> str: +def rfc_822_date_format(date: datetime.date | datetime.datetime) -> str: """ Formats a date into RFC 822 format. """ diff --git a/tna_utilities/flask/talisman.py b/tna_utilities/flask/talisman.py index 6f8b25b..01e3aa5 100644 --- a/tna_utilities/flask/talisman.py +++ b/tna_utilities/flask/talisman.py @@ -127,24 +127,26 @@ def _force_https_redirect(self): flask.request.headers.get("X-Forwarded-Proto", "http") == "https", ] - if self.force_https and not any(criteria): - if flask.request.url.startswith("http://"): - parsed = urlparse(flask.request.url) - target = urlunparse( - ( - "https", - parsed.netloc, - parsed.path, - parsed.params, - parsed.query, - parsed.fragment, - ) + if ( + self.force_https + and not any(criteria) + and flask.request.url.startswith("http://") + ): + parsed = urlparse(flask.request.url) + target = urlunparse( + ( + "https", + parsed.netloc, + parsed.path, + parsed.params, + parsed.query, + parsed.fragment, ) - code = 302 - if self.force_https_permanent: - code = 301 - r = flask.redirect(target, code=code) - return r + ) + code = 302 + if self.force_https_permanent: + code = 301 + return flask.redirect(target, code=code) return None diff --git a/tna_utilities/number.py b/tna_utilities/number.py index d50671f..9451ea7 100644 --- a/tna_utilities/number.py +++ b/tna_utilities/number.py @@ -1,19 +1,18 @@ import math import re -from typing import Union def numberish( - value: Union[float, int], + value: float | int, simple_units: bool = False, - prefix_text: Union[str, tuple[str, str]] = "About ", + prefix_text: str | tuple[str, str] = "About ", ) -> str: """ Convert a number into a human-readable string with appropriate units. """ if not isinstance(value, (int, float)): - raise ValueError("Value must be an integer or float") + raise TypeError("Value must be an integer or float") if value == 0: return "None" units = [ @@ -29,12 +28,12 @@ def numberish( return f"{int(base_value)}{unit}" base_value_rounded = round( base_value, - -int(math.floor(math.log10(abs(base_value)))) + 1, + -(math.floor(math.log10(abs(base_value)))) + 1, ) if base_value_rounded == base_value: prefix_text = "" elif isinstance(prefix_text, tuple): - if len(prefix_text) != 2: + if not prefix_text[0] or not prefix_text[1]: raise ValueError("prefix_text tuple must have exactly two elements") if not all(isinstance(pt, str) for pt in prefix_text): raise ValueError( @@ -48,13 +47,13 @@ def numberish( return str(int(value)) -def pretty_file_size(bytes, simplify=True): - if not isinstance(bytes, int): - raise ValueError("file_size must be an integer") +def pretty_file_size(filesize_bytes, simplify=True): + if not isinstance(filesize_bytes, int): + raise TypeError("filesize_bytes must be an integer") byte_unit = 1000 suffixes = ["B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"] i = 0 - prettified_file_size = bytes + prettified_file_size = filesize_bytes while prettified_file_size >= byte_unit and i < len(suffixes) - 1: prettified_file_size /= byte_unit i += 1 diff --git a/tna_utilities/security.py b/tna_utilities/security.py index 45f7bf0..51344d4 100644 --- a/tna_utilities/security.py +++ b/tna_utilities/security.py @@ -73,9 +73,7 @@ def _process_sources(self, *values: str | list[str]) -> list[str]: processed_values = [src for src in processed_values if src] # Remove duplicates while preserving order - processed_values = list(dict.fromkeys(processed_values)) - - return processed_values + return list(dict.fromkeys(processed_values)) def add_directive( self, directive: str, *values: str | list[str], omit_self=False, replace=False @@ -359,10 +357,7 @@ def sandbox(self, value: str | None = None) -> "CspGenerator": "allow-top-navigation-by-user-activation", "allow-top-navigation-to-custom-protocols", ] - if value is not None and value in values: - sources = [value] - else: - sources = [] + sources = [value] if value is not None and value in values else [] self.directives["sandbox"] = sources return self diff --git a/tna_utilities/url.py b/tna_utilities/url.py index d014280..ba31f66 100644 --- a/tna_utilities/url.py +++ b/tna_utilities/url.py @@ -17,10 +17,10 @@ def __init__(self, args=None) -> None: elif args is not None: try: args_lists = args.lists() - except AttributeError: + except AttributeError as e: raise AttributeError( "args must be an ImmutableMultiDict (Django), a QueryDict (Flask) object or an iterable of (key, [values]) tuples" - ) + ) from e self.args = list(args_lists) else: self.args = [] @@ -30,10 +30,7 @@ def parameter_exists(self, parameter) -> bool: Check if a parameter exists in the query parameters. """ - for key, _ in self.args: - if key == parameter: - return True - return False + return any(key == parameter for key, _ in self.args) def parameter_values(self, parameter: str) -> list: """ @@ -54,7 +51,7 @@ def add_parameter( Raises a ValueError if the parameter already exists. """ - for key, vals in self.args: + for key, _vals in self.args: if key == parameter: raise ValueError(f"Parameter '{parameter}' already exists") if not isinstance(values, list): @@ -83,7 +80,7 @@ def remove_parameter(self, parameter: str) -> "QueryStringTransformer": Raises a KeyError if the parameter does not exist. """ - for index, (key, vals) in enumerate(self.args): + for index, (key, _vals) in enumerate(self.args): if key == parameter: del self.args[index] return self @@ -129,9 +126,8 @@ def toggle_parameter_value( str_value = str(value) if str_value in values: values.remove(str_value) - else: - if str_value not in values: - values.append(str_value) + elif str_value not in values: + values.append(str_value) return self raise KeyError(f"Parameter '{parameter}' does not exist") From c838fd005555b844d2171df2112388324af9e659 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 14 May 2026 13:07:13 +0100 Subject: [PATCH 2/4] Update ruff version in checkformat script to use test image --- tasks/checkformat.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tasks/checkformat.sh b/tasks/checkformat.sh index 63db9d9..d7fae12 100755 --- a/tasks/checkformat.sh +++ b/tasks/checkformat.sh @@ -1,3 +1,4 @@ #!/bin/bash -docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview checkformat +# TODO: Change ruff version back to preview once the Docker image has been released +docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff checkformat From b67bf4fcee8afacecb16cbfae3c510090bf94ed3 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 14 May 2026 14:23:18 +0100 Subject: [PATCH 3/4] Remove experimental and non-spec CSP directives --- tests/test_security.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index affb9f9..3b22169 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -269,7 +269,6 @@ def test_add_directive_sources(self): ("base-uri", "base_uri"), ("child-src", "child_src"), ("connect-src", "connect_src"), - # ("fenced-frame-src", "fenced_frame_src"), # Experimental ("font-src", "font_src"), ("form-action", "form_action"), ("frame-ancestors", "frame_ancestors"), @@ -285,8 +284,11 @@ def test_add_directive_sources(self): ("style-src", "style_src"), ("style-src-attr", "style_src_attr"), ("style-src-elem", "style_src_elem"), - # ("trusted-types", "trusted_types"), # Not technically part of the CSP spec ("worker-src", "worker_src"), + # Experimental + # ("fenced-frame-src", "fenced_frame_src"), # noqa: ERA001 + # Not technically part of the CSP spec + # ("trusted-types", "trusted_types"), # noqa: ERA001 ] for directive, method in directives: generator = CspGenerator() From 75baa8dc4a698a6bc4d6649f2faab41a8afb62c2 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 14 May 2026 17:22:12 +0100 Subject: [PATCH 4/4] Update checkformat and format scripts to use preview Docker image --- tasks/checkformat.sh | 3 +-- tasks/format.sh | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tasks/checkformat.sh b/tasks/checkformat.sh index d7fae12..63db9d9 100755 --- a/tasks/checkformat.sh +++ b/tasks/checkformat.sh @@ -1,4 +1,3 @@ #!/bin/bash -# TODO: Change ruff version back to preview once the Docker image has been released -docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff checkformat +docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview checkformat diff --git a/tasks/format.sh b/tasks/format.sh index e07833b..dbbedf0 100755 --- a/tasks/format.sh +++ b/tasks/format.sh @@ -1,4 +1,3 @@ #!/bin/bash -# TODO: Change ruff version back to preview once the Docker image has been released -docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:ruff format +docker run --rm -v "$(pwd)":/app/ ghcr.io/nationalarchives/tna-python-dev:preview format