-
Notifications
You must be signed in to change notification settings - Fork 0
v2.0.5 - Improve test coverage and typing, remove deprecated functions #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Enforce strict mypy typing across package and tests. Add typed helpers (JSONType, AuthArtifacts, PictureMetadata). Remove legacy Device wrappers (fetch_pictures/get_pictures/download_photo/get_picture/take_front_photo/take_rear_photo). Add Device.get_picture_metadata, tests, docs, and migration notes; fix linting/CI issues.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the codebase with strict typing enforcement and improved test coverage while removing deprecated API methods. However, there's a critical version mismatch: the PR is titled "v2.0.5" but the code changes bump the version to 2.0.6.
Key Changes:
- Enforced strict mypy typing across the entire codebase including tests
- Added new
Device.get_picture_metadata()method for filtered metadata access - Removed deprecated compatibility wrapper methods (breaking change)
- Added extensive test coverage for edge cases and error paths
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bumped to 2.0.6 (conflicts with PR title); removed mypy test directory overrides |
| fmd_api/_version.py | Version string updated to 2.0.6 |
| fmd_api/types.py | New file introducing JSONType alias and TypedDicts (AuthArtifacts, PictureMetadata) |
| fmd_api/models.py | Updated type annotations to use JSONType instead of Any |
| fmd_api/device.py | Removed 6 deprecated methods; added get_picture_metadata(); improved type annotations |
| fmd_api/client.py | Enhanced type annotations throughout with ssl.SSLContext, JSONType, and runtime type narrowing |
| tests/unit/test_resume.py | Added tests for artifact validation with invalid types and non-int session_duration |
| tests/unit/test_device.py | Added test for picture blob/metadata filtering; updated to use new method names |
| tests/unit/test_deprecated.py | Deleted entire file (86 lines) as deprecated methods were removed |
| tests/unit/test_coverage_improvements.py | Added 280+ lines of edge case tests for improved coverage |
| docs/release/v2.0.5.md | New release notes document (contradictory migration section) |
| docs/MIGRATE_FROM_V1.md | Updated with v2.0.5 breaking change notes |
| docs/release/v2.0.0_merge_request.md | Updated TestPyPI deployment notes |
| README.md | Updated examples, added picture metadata usage, noted breaking changes |
| .github/workflows/publish-testpypi.yml | Removed entire TestPyPI workflow file |
| coverage.xml | Updated coverage report showing 100% line coverage |
| .coverage | Binary coverage database file updated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [project] | ||
| name = "fmd_api" | ||
| version = "2.0.5" | ||
| version = "2.0.6" |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version mismatch: The PR title and description state this is "v2.0.5" but the code changes bump the version to "2.0.6". Either the PR metadata should be updated to reflect v2.0.6, or the version numbers in the code should be corrected to 2.0.5.
| version = "2.0.6" | |
| version = "2.0.5" |
| @@ -1 +1 @@ | |||
| __version__ = "2.0.5" | |||
| __version__ = "2.0.6" | |||
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version mismatch: This version string is "2.0.6" but the PR title and description indicate this is "v2.0.5". The version should be consistent with the PR metadata.
| __version__ = "2.0.6" | |
| __version__ = "2.0.5" |
| Migration | ||
| - No breaking API changes. Prefer `get_picture_metadata` where you only need metadata. | ||
|
|
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent messaging in "Migration" section: Lines 48-49 state "No breaking API changes" but the entire release notes document describes significant breaking changes (removal of deprecated methods). This section should either be removed or updated to accurately reflect the breaking changes, consistent with the "Notes for integrators (breaking change)" section above.
| Migration | |
| - No breaking API changes. Prefer `get_picture_metadata` where you only need metadata. | |
| """Get raw picture blobs (usually a list of dicts returned by the server). | ||
| The client may return lists of mixed JSON values; coerce/filter here so callers | ||
| (and the annotated return type) are guaranteed a list of dicts. |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistency between docstring and implementation: The docstring on lines 89-92 states that callers are "guaranteed a list of dicts", but the actual return type is List[JSONType] which includes strings, ints, bools, etc. The implementation on lines 94-97 also preserves all raw server values including strings. The docstring should be updated to accurately reflect that this method returns all raw server values (strings, dicts, etc.) without filtering, not just dicts.
| """Get raw picture blobs (usually a list of dicts returned by the server). | |
| The client may return lists of mixed JSON values; coerce/filter here so callers | |
| (and the annotated return type) are guaranteed a list of dicts. | |
| """Get raw picture blobs returned by the server (may include strings, dicts, etc.). | |
| The client may return lists of mixed JSON values; this method preserves all raw values | |
| (strings, dicts, etc.) without filtering. Callers should not expect only dicts. |
| pip install --pre --index-url https://test.pypi.org/simple/ \ | ||
| --extra-index-url https://pypi.org/simple/ fmd_api | ||
| ``` | ||
| <!-- Pre-release via TestPyPI removed. Use stable releases from PyPI or GitHub Releases for pre-release artifacts --> |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Incomplete sentence in comment: The comment ends abruptly with "Pre-release via TestPyPI removed." Consider completing the sentence or making it more grammatically correct, for example: "Pre-release testing via TestPyPI has been removed. Use stable releases from PyPI or GitHub Releases for pre-release artifacts."
| <!-- Pre-release via TestPyPI removed. Use stable releases from PyPI or GitHub Releases for pre-release artifacts --> | |
| <!-- Pre-release testing via TestPyPI has been removed. Please use stable releases from PyPI or GitHub Releases for pre-release artifacts. --> |
| await client.export_data_zip(output_path, include_pictures=False) | ||
|
|
||
| # locations.json should contain an error entry for index 0 | ||
| import json |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module json is redundant, as it was previously imported on line 6.
| import json |
|
|
||
| # locations.json should contain an error entry for index 0 | ||
| import json | ||
| import zipfile |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module zipfile is redundant, as it was previously imported on line 8.
| import zipfile |
|
|
||
| await client.export_data_zip(output_path, include_pictures=True) | ||
|
|
||
| import json |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module json is redundant, as it was previously imported on line 6.
| import json |
| await client.export_data_zip(output_path, include_pictures=True) | ||
|
|
||
| import json | ||
| import zipfile |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module zipfile is redundant, as it was previously imported on line 8.
| import zipfile |
|
|
||
| try: | ||
| import tempfile | ||
| import zipfile |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module zipfile is redundant, as it was previously imported on line 8.
| import zipfile |
Release notes — v2.0.5
Released: 2025-11-21
This release focuses on two related improvements: full strict typing for the codebase
and a clearer, more strongly-typed picture API. Important: this release removes several
deprecated compatibility wrappers from the Device API — this is a breaking change.
Highlights
Enforced strict typing across the entire repository
mypyis now run in strict mode over the whole project (tests included).fmd_api/types.pycentralizes JSON/response types and the newPictureMetadataTypedDict.proper return annotations and runtime narrowing where needed.
Device picture APIs improved
Device.get_picture_metadata(num_to_get: int = -1) -> List[PictureMetadata]server exposes them.
release removes the old backward-compatible wrappers:
fetch_pictures(),get_pictures(),download_photo(),get_picture(),take_front_photo(),and
take_rear_photo()are no longer available. Calls must be migrated to thecanonical APIs:
get_picture_blobs(),decode_picture(),take_front_picture(),and
take_rear_picture().Why this change?
contributors and users better IDE/typing support.
get_picture_metadatamethod makes it convenient to work with structuredmetadata where available while preserving raw data semantics for older servers and
decoding use-cases.
Notes for integrators (breaking change)
Device.fetch_pictures,Device.get_pictures,Device.download_photo,Device.get_picture,Device.take_front_photo,Device.take_rear_photohave been removed in v2.0.5.fetch_pictures()/get_pictures()->get_picture_blobs()download_photo()/get_picture()->decode_picture()take_front_photo()->take_front_picture()andtake_rear_photo()->take_rear_picture()If you were relying on
get_picture_blobsreturning only dicts, useget_picture_metadata()to receiveonly metadata entries (dicts) where available.
Migration
get_picture_metadatawhere you only need metadata.Acknowledgements
these changes safe across the codebase.