Modernize library: packaging, typed API, bug fixes, tests, CI, uv, and MIT license#1
Merged
Merged
Conversation
- Add pyproject.toml (hatchling) with metadata, ruff and pytest config - Restructure into snake_case modules (vapix_api, ptz_control, geolocation_api) with deprecated CamelCase shims for backward compat - Fix bugs: request timeout was never applied (requests ignores session.timeout), rename_preset_number sent number/name swapped, unsupported HTTP methods crashed with UnboundLocalError, geolocation XML parsing failed on namespaced responses and printed debug output - Add typed exception hierarchy (VapixError, VapixRequestError, VapixAuthenticationError, VapixResponseError) - Add HTTPS support, basic-auth option, configurable camera channel, context-manager support, and a get_parameters() helper (param.cgi) - Robust key=value and XML response parsing with clear errors - Populate __init__.py with public exports and __version__ - Add mocked pytest suite (35 tests, no camera required) - Add GitHub Actions CI (ruff + pytest on Python 3.9-3.13) - Rewrite README with accurate usage, options, and migration notes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Biy4fbT2vTaDATsMcrLfm
- Add MIT LICENSE (copyright 2023 derens99) and reference it in pyproject.toml via SPDX expression + license-files - Add [dependency-groups] dev group and commit uv.lock for reproducible dev environments with uv sync - CI now installs via astral-sh/setup-uv with caching, runs lint/tests through uv, and builds the distribution - README development docs updated for the uv workflow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Biy4fbT2vTaDATsMcrLfm
Correctness fixes from an 8-angle review of the modernization diff:
- Remove the deprecated CamelCase shim modules: importing
vapix_python.VapixAPI rebound the package attribute from the class to
the shim module (standard submodule-import behavior), so mixing old
and new import styles in one process made the class un-callable.
A loud ModuleNotFoundError beats silent namespace corruption.
- VapixRequestError now also subclasses requests.RequestException so
pre-0.2.0 'except requests.RequestException' handlers keep working;
HTTP 403 (insufficient privileges) now raises VapixAuthenticationError
alongside 401.
- ptz_enabled(channel) now sends info=1&camera=<channel>; the channel
number was previously sent as the info value and never selected a
channel.
- PTZ commands rejected by the camera with an 'Error: ...' body (HTTP
200 on some firmware) now raise VapixResponseError instead of
silently succeeding.
- geolocation set_position no longer fails on firmware that answers a
successful set with an empty or plain-text body; ValidPosition/
ValidHeading accept non-canonical truthy values ('1', 'yes') again,
matching 0.1.x leniency.
- get_parameters forwards an explicit empty group instead of dropping
it; restore the documented .password attribute.
Structure/cleanup from the same review:
- Transport is now endpoint-agnostic: ptz.cgi base args (camera/html/
timestamp) moved into PTZControl._command; _send_request_vanilla and
the base_args flag are gone.
- Shared key=value parser in _parsing.py (was duplicated in vapix_api
and ptz_control); level setters collapsed into _set_level; geolocation
XML parsed in a single pass; GeoPosition TypedDict return type.
- Drop requirements.txt and the duplicated optional-dependencies dev
list (dependency-groups is the single source); remove dead ruff
ignores; pin hatchling>=1.27 for the PEP 639 license fields.
- Tests updated and extended: 48 passing (was 35), covering the new
behaviors above.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Biy4fbT2vTaDATsMcrLfm
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Full modernization of the library (v0.2.0): proper packaging, typed public API, real bug fixes, a mocked test suite, CI, uv-based tooling, and an MIT license. All changes were run through an 8-angle code review with confirmed findings fixed, plus an end-to-end correctness check against a fake Axis camera server (real HTTP + digest auth).
Bug fixes
session.timeoutis ignored byrequests, so every camera call could hang forever. Timeouts are now passed per request and enforced.rename_preset_numbersent its two parameters swapped (renameserverpresetnogot the name,newnamegot the number).ptz_enabled(channel)never selected the channel — the channel number was sent as theinfovalue; now sendsinfo=1&camera=<channel>.Error: ...with HTTP 200 now raisesVapixResponseError.set.cgi, accepts non-canonical validity flags (1,yes), no strayprint()output, and unsupported HTTP methods no longer crash withUnboundLocalError.Modernization
pyproject.toml(hatchling) with full metadata; Python 3.9–3.13; version 0.2.0vapix_api,ptz_control,geolocation_api); public exports fromvapix_python(from vapix_python import VapixAPI)VapixRequestErrorstill subclassesrequests.RequestExceptionso 0.1.xexcepthandlers keep working; 401 and 403 map toVapixAuthenticationErrorget_parameters()helper forparam.cgiptz.cgibase args live in the PTZ layer, not the shared request pathLICENSEwired in via PEP 639 fields (hatchling>=1.27)uv.lock,[dependency-groups]dev group, README dev workflowuv sync→ ruff → pytest →uv buildon Python 3.9–3.13Breaking changes (documented in the README migration section)
from vapix_python.VapixAPI import VapixAPI) were removed: importing them rebound the package attribute from the class to the module, breakingfrom vapix_python import VapixAPIelsewhere in the same process. They now fail loudly instead of corrupting the namespace.Testing
ruff checkclean; sdist + wheel build successfully🤖 Generated with Claude Code
https://claude.ai/code/session_011Biy4fbT2vTaDATsMcrLfm
Generated by Claude Code