Use us_customary instead of imperial for Home Assistant consistency by eman · Pull Request #65 · eman/nwp500-python

eman · 2026-01-26T04:31:24Z

Overview

This PR removes all references to 'imperial' terminology and exclusively uses Home Assistant's official term 'us_customary' for measurement systems throughout the library.

Changes

Removed 'imperial' from all Literal type hints (now only accepts 'metric' and 'us_customary')
Removed imperial-to-us_customary mapping logic (no longer needed since 'imperial' is not accepted)
Updated all docstrings to reference only 'us_customary'
Updated unit_system_to_temperature_type() to handle only 'us_customary'
Updated CLI to use 'us_customary' instead of 'imperial'
Updated all test references to use 'us_customary'
Added UnitSystemType type alias for cleaner code
Fixed linting errors (line length, syntax)

Rationale

Since this version hasn't been deployed yet, clean up the API to exclusively use Home Assistant's official term 'us_customary' rather than maintaining backward compatibility with 'imperial'.

API Changes

Before:

set_unit_system("metric" | "imperial" | None)

After:

set_unit_system("metric" | "us_customary" | None)

Backward Compatibility

None - this is a breaking change, but acceptable since it hasn't been released yet and makes the API cleaner and more aligned with Home Assistant standards.

Changed all references from 'imperial' to 'us_customary' to align with Home Assistant's official terminology. 'imperial' is maintained as a backward-compatible alias that maps to 'us_customary'. Changes: - set_unit_system() accepts 'us_customary', 'metric', or 'imperial' (deprecated) - get_unit_system() returns 'us_customary' or 'metric' - 'imperial' input automatically maps to 'us_customary' - All docstrings updated to reference 'us_customary' - Type hints support both for backward compatibility Rationale: Home Assistant uses 'us_customary' as the official term for the imperial measurement system. By using the same terminology, we ensure consistency and make integration with HA seamless. Backward Compatibility: - Existing code passing 'imperial' will continue to work - Internally converted to 'us_customary' for consistency - No breaking changes for users

Copilot

Pull request overview

This PR updates the codebase to use 'us_customary' instead of 'imperial' for unit system terminology, aligning with Home Assistant's official conventions while maintaining backward compatibility through automatic aliasing.

Changes:

Modified set_unit_system() to accept 'us_customary' as the primary term and automatically map 'imperial' to 'us_customary' for backward compatibility
Updated get_unit_system() return type to exclude 'imperial' since it's now internally converted
Updated all docstrings, type hints, and comments across the codebase to reference 'us_customary' as the standard term

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/nwp500/unit_system.py	Core unit system logic updated with 'us_customary' terminology and imperial-to-us_customary mapping
src/nwp500/mqtt/subscriptions.py	Type hints updated to include 'us_customary' in MqttSubscriptionManager
src/nwp500/mqtt/client.py	Type hints and docstrings updated in MqttClient for 'us_customary'
src/nwp500/auth.py	Type hints and docstrings updated in NavienAuthClient for 'us_customary'
src/nwp500/api_client.py	Type hints and docstrings updated in NavienAPIClient for 'us_customary'

Comments suppressed due to low confidence (1)

src/nwp500/unit_system.py:1

The stored unit_system should not include 'imperial' since it's immediately converted to 'us_customary' in set_unit_system(). The type hint should be Literal['metric', 'us_customary'] | None to reflect the internal state after conversion.

"""Unit system management for temperature, flow rate, and volume conversions.

Copilot · 2026-01-26T04:33:59Z

src/nwp500/mqtt/client.py


        self._auth_client = auth_client
-        self._unit_system: Literal["metric", "imperial"] | None = unit_system
+        self._unit_system: Literal["metric", "us_customary", "imperial"] | None = unit_system


The stored unit_system should not include 'imperial' in its type since set_unit_system() converts 'imperial' to 'us_customary'. The type hint should be Literal['metric', 'us_customary'] | None to reflect the actual stored values after conversion.

Suggested change

self._unit_system: Literal["metric", "us_customary", "imperial"] | None = unit_system

normalized_unit_system: Literal["metric", "us_customary"] | None

if unit_system == "imperial":

normalized_unit_system = "us_customary"

else:

normalized_unit_system = unit_system

self._unit_system: Literal["metric", "us_customary"] | None = normalized_unit_system

Since this hasn't been deployed yet, remove support for 'imperial' as input. Use only 'us_customary' consistently across the library. Changes: - set_unit_system() accepts only 'metric' or 'us_customary' (not 'imperial') - get_unit_system() returns 'metric' or 'us_customary' - Removed 'imperial' from all Literal type hints - Removed 'imperial' mapping logic - Updated all docstrings to reference only 'us_customary' - Updated unit_system_to_temperature_type to handle only 'us_customary' - Updated CLI to use 'us_customary' instead of 'imperial' - Updated all test references This ensures clean, consistent terminology aligned with Home Assistant's official 'us_customary' term.

- Fixed unit_system.py line 27: missing # on comment line - Added UnitSystemType type alias to mqtt/client.py and mqtt/subscriptions.py - Replaced long Literal type hints with UnitSystemType alias (fixes line length) - Fixed long docstring line in is_metric_preferred() return description All lines now within 80 character limit. Syntax errors resolved.

- Moved local imports before TYPE_CHECKING block (fixes E402 errors) - Split long docstring line in subscriptions.py (fixes E501 error) - All imports now properly ordered - All lines within 80 character limit

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot · 2026-01-26T05:01:03Z

src/nwp500/mqtt/subscriptions.py

 from ..unit_system import set_unit_system
 from .utils import redact_topic, topic_matches_pattern

+# Type alias for unit system preference
+UnitSystemType = Literal["metric", "us_customary"] | None


The UnitSystemType alias is defined in two separate files (subscriptions.py and client.py). This duplication should be avoided by defining the type alias once in a shared location (e.g., unit_system.py where the unit system functionality is centralized) and importing it where needed.

Suggested change

from ..unit_system import set_unit_system

from .utils import redact_topic, topic_matches_pattern

# Type alias for unit system preference

UnitSystemType = Literal["metric", "us_customary"] | None

from ..unit_system import UnitSystemType, set_unit_system

from .utils import redact_topic, topic_matches_pattern

Copilot · 2026-01-26T05:01:04Z

src/nwp500/mqtt/client.py

+# Type alias for unit system preference
+UnitSystemType = Literal["metric", "us_customary"] | None


This is a duplicate definition of UnitSystemType that also appears in subscriptions.py. Define the type alias once in unit_system.py and import it in both locations to maintain a single source of truth.

- Defined UnitSystemType type alias in unit_system.py (single source of truth) - Removed duplicate definitions from mqtt/client.py and mqtt/subscriptions.py - Import UnitSystemType from unit_system module where needed This follows DRY principle and maintains type consistency across the codebase.

eman · 2026-01-26T05:04:47Z

Regarding the suggestion about normalizing unit_system values in mqtt/client.py:

Since we've now removed 'imperial' from the accepted values entirely (it's no longer in the UnitSystemType definition), the type system already prevents any 'imperial' values from being passed to init. The function signature:

unit_system: UnitSystemType = None

where UnitSystemType = Literal["metric", "us_customary"] | None

ensures only valid values can be passed. Type checkers will catch any attempts to pass 'imperial' at static analysis time.

However, I've addressed the DRY issue by consolidating the UnitSystemType alias to a single definition in unit_system.py and importing it in both mqtt/client.py and mqtt/subscriptions.py. This was a good catch!

After consolidating UnitSystemType to unit_system.py, the Literal type is no longer directly used in subscriptions.py imports. Remove the unused import to fix pyright error.

- Removed unused Literal import from mqtt/client.py - Sorted imports alphabetically per ruff isort rules - UnitSystemType now comes before set_unit_system in imports - All ruff linting errors now resolved - Verified locally with tox -e lint before pushing

eman requested a review from Copilot January 26, 2026 04:33

Copilot AI reviewed Jan 26, 2026

View reviewed changes

eman added 3 commits January 25, 2026 20:46

Fix import ordering and line length issues

cc3efc3

- Moved local imports before TYPE_CHECKING block (fixes E402 errors) - Split long docstring line in subscriptions.py (fixes E501 error) - All imports now properly ordered - All lines within 80 character limit

eman requested a review from Copilot January 26, 2026 05:00

Copilot AI reviewed Jan 26, 2026

View reviewed changes

eman added 2 commits January 25, 2026 21:07

Remove unused Literal import from subscriptions.py

6300715

After consolidating UnitSystemType to unit_system.py, the Literal type is no longer directly used in subscriptions.py imports. Remove the unused import to fix pyright error.

eman merged commit 8ab8648 into main Jan 26, 2026
7 checks passed

eman deleted the fix/remove-strict-token-check-in-init branch January 26, 2026 05:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use us_customary instead of imperial for Home Assistant consistency#65

Use us_customary instead of imperial for Home Assistant consistency#65
eman merged 7 commits intomainfrom
fix/remove-strict-token-check-in-init

eman commented Jan 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

eman commented Jan 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        self._unit_system: Literal["metric", "us_customary", "imperial"] | None = unit_system
+        normalized_unit_system: Literal["metric", "us_customary"] | None
+        if unit_system == "imperial":
+            normalized_unit_system = "us_customary"
+        else:
+            normalized_unit_system = unit_system
+        self._unit_system: Literal["metric", "us_customary"] | None = normalized_unit_system

		# Type alias for unit system preference
		UnitSystemType = Literal["metric", "us_customary"] \| None

Conversation

eman commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Rationale

API Changes

Backward Compatibility

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

eman commented Jan 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eman commented Jan 26, 2026 •

edited

Loading