prusalink: tighten typing on job.filename and job.finish sensors#170481
Closed
heikkih wants to merge 7 commits into
Closed
prusalink: tighten typing on job.filename and job.finish sensors#170481heikkih wants to merge 7 commits into
heikkih wants to merge 7 commits into
Conversation
3.0.0 changes that motivate this bump: - `get_job()` now returns `JobInfo | None` (returns None on 204 instead of casting an empty dict to JobInfo) — clarifies the no-job case - `PrinterInfo` fields migrated from `T | None` to `NotRequired[T]` to match what the API actually returns - `jobId` parameter renamed to `job_id` on cancel/pause/resume/ continue_job; HA calls these positionally, no impact - `py.typed` marker added (PEP 561) — pyprusalink's typing now applies to HA's mypy run; subsequent commits fix the latent issues this surfaces Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes that mypy strict now requires after pyprusalink 3.0.0 added its py.typed marker: - `JobUpdateCoordinator` now binds `[JobInfo | None]` to reflect that pyprusalink's `get_job()` returns `None` on HTTP 204 when no job is running. `_fetch_data` return type follows. - `_get_update_interval(data: T)` becomes `data: T | None` because the base class is called once from `__init__` (line 57) with `None` before the first fetch. The parameter is unused by the base today, but kept on the signature for subclasses that may override based on payload state (e.g. a future transfer coordinator polling faster while a transfer is active). Documented in a docstring. The `T` TypeVar gains `JobInfo | None` as a valid binding so `JobUpdateCoordinator[JobInfo | None]` type-checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…orkaround
`version.get("original", "")` looks up a field that is not declared on
the `VersionInfo` TypedDict because `original` is an undocumented field
returned only by older standalone PrusaLink 0.7.2 builds on MK3 and
MK2.5 printers. With pyprusalink 3.0.0's py.typed marker active, mypy
narrows the .get() default-fallback return type to `object | str` and
correctly objects to `.startswith` on `object`.
Cast to `str` since we know what the printer actually returns when the
field is present. The runtime behaviour is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pyprusalink 3.0.0 changes `get_job()` to return `None` on HTTP 204 when no job is running, instead of casting an empty dict to JobInfo. The job coordinator's `data` therefore becomes `None` whenever the printer is not running a job. Every entity's `available_fn` lambda expects a dict (uses `.get()` or direct indexing). Without this guard, accessing the job-coordinator- backed entities while the printer is idle raised AttributeError. Short-circuit at the base entity so the lambdas never see `None`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `mock_job_api_idle` fixture was returning `{}` — the value 2.x's
`get_job()` produced on HTTP 204. With pyprusalink 3.0.0 the method
returns `None` instead, so the mock needs to match. With the new mock,
the test suite exercises the `coordinator.data is None` path in
`PrusaLinkEntity.available` introduced in the previous commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rker pyprusalink 3.0.0 ships the PEP 561 `py.typed` marker so the library's declared types now apply to HA's mypy run. Five issues surfaced — two are real fixes, two are pre-existing debt routed through `# type: ignore` for a focused followup, and one is a redundant cast we can just drop. - printer.state sensor: rearrange `cast(str, …).lower()` parentheses so the cast wraps the value before `.lower()` is called. Runtime is unchanged; mypy now sees `str.lower()` instead of `PrinterState.lower()`. - printer.telemetry.material sensor: the `LegacyPrinterStatus.telemetry` field is `LegacyPrinterTelemetry | None`, so `data["telemetry"] ["material"]` could KeyError at runtime when `telemetry` was None. Real latent bug: add `available_fn` that gates on `telemetry`, and cast the inner Optional to satisfy mypy. - job.filename and job.finish sensors: `available_fn` already gates against None / Optional, but mypy doesn't see through the guarantee into the lambdas. `# type: ignore` with a TODO comment so a focused followup PR can replace them with proper narrowing. - info.min_extrusion_temp sensor: `min_extrusion_temp` was migrated from `int | None` to `NotRequired[int]` in pyprusalink 3.0.0; the cast that previously narrowed `int | None` to `int` is now redundant and can be dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to home-assistant#170480, addressing the two `# type: ignore` comments that PR added to the `job.filename` and `job.finish` sensors. Both lambdas are guarded at runtime by `available_fn` (verifying `data["file"]` and `data["time_remaining"]` are not None respectively), but mypy doesn't follow the guarantee from `available_fn` into `value_fn`. Switch to the inner-cast pattern already used on the `printer.telemetry.material` sensor: - `job.filename`: `cast(JobFilePrint, data["file"])["display_name"]` narrows `JobFilePrint | None` so the index is type-safe. - `job.finish`: `cast(int, data["time_remaining"])` narrows `int | None` so it can be passed to `timedelta(seconds=...)`. Runtime behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Closing in favour of folding into #170480. Copilot's review on #170480 pointed out (correctly) that the typing fixes should not be split across two PRs — the two |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the PrusaLink integration to use pyprusalink==3.0.0 and adjusts integration/runtime + typing behavior around nullable job payloads and sensor value functions.
Changes:
- Bump
pyprusalinkto 3.0.0 and update tests to reflectget_job()returningNonewhen idle. - Make the job coordinator payload nullable (
JobInfo | None) and guard entity availability when coordinator data isNone. - Tighten typing in several sensor/config-flow lambdas via inner
cast()patterns.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/prusalink/conftest.py | Updates idle job mock to return None for get_job() (pyprusalink 3.x behavior). |
| requirements_test_all.txt | Bumps pyprusalink version pin used for test environments. |
| requirements_all.txt | Bumps pyprusalink version pin used for full dependency set. |
| homeassistant/components/prusalink/sensor.py | Reworks a few sensor lambdas to use inner casts and adds availability guard for legacy telemetry material. |
| homeassistant/components/prusalink/manifest.json | Bumps the integration requirement to pyprusalink==3.0.0. |
| homeassistant/components/prusalink/entity.py | Prevents calling available_fn when coordinator data is None. |
| homeassistant/components/prusalink/coordinator.py | Allows nullable job coordinator data and adapts update interval signature for init-time None. |
| homeassistant/components/prusalink/config_flow.py | Casts undocumented original version field to satisfy type checking. |
Comment on lines
121
to
+129
| @pytest.fixture | ||
| def mock_job_api_idle() -> Generator[dict[str, Any]]: | ||
| """Mock PrusaLink job API having no job.""" | ||
| resp = {} | ||
| with patch("pyprusalink.PrusaLink.get_job", return_value=resp): | ||
| yield resp | ||
| def mock_job_api_idle() -> Generator[None]: | ||
| """Mock PrusaLink job API having no job. | ||
|
|
||
| pyprusalink >= 3.0.0 returns `None` from `get_job()` on HTTP 204 when | ||
| no job is running, rather than an empty dict as in 2.x. | ||
| """ | ||
| with patch("pyprusalink.PrusaLink.get_job", return_value=None): | ||
| yield None |
Comment on lines
12
to
15
| "integration_type": "device", | ||
| "iot_class": "local_polling", | ||
| "requirements": ["pyprusalink==2.2.0"] | ||
| "requirements": ["pyprusalink==3.0.0"] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Follow-up to #170480 (depends on it merging first).
That PR added two
# type: ignorecomments on thejob.filenameandjob.finishsensors withTODOs for a focused follow-up — this is the follow-up. Both lambdas are guarded at runtime byavailable_fn, but mypy doesn't follow the guarantee intovalue_fn. Switch to the inner-cast pattern that's already used on theprinter.telemetry.materialsensor in the same file.job.filename—cast(JobFilePrint, data["file"])["display_name"]narrows theJobFilePrint | Noneso["display_name"]type-checks.job.finish—cast(int, data["time_remaining"])narrows theint | Noneso it can be passed totimedelta(seconds=...).Why now
Keeps the bump PR focused on what the 3.0.0 release actually requires (runtime fixes + typing surfaced by the new
py.typedmarker), and addresses the typing debt as its own reviewable change.No runtime impact
cast()is erased at runtime; existing tests already cover both lambdas in the happy path (file/time_remaining present) and theavailable_fnshort-circuit when they're absent. No new tests needed.Type of change
Checklist
pytest tests/components/prusalink/— 34 passedprek run --files homeassistant/components/prusalink/sensor.py— all hooks green (mypy, ruff, pylint, hassfest)