Skip to content

fix: correct return types of image comparison and get_events - #1347

Open
MattL1989 wants to merge 1 commit into
appium:masterfrom
MattL1989:fix/images-log-event-return-types
Open

MattL1989 wants to merge 1 commit into
appium:masterfrom
MattL1989:fix/images-log-event-return-types

Conversation

@MattL1989

Copy link
Copy Markdown
Contributor

Problem

Three methods have return annotations that do not match what the server sends:

  • find_image_occurrence() and get_images_similarity() are annotated -> dict[str, bytes | dict]. The images plugin (packages/images-plugin/lib/compare.ts) returns score as a number and converts visualization to a base64 string. When the multiple option is set, find_image_occurrence() returns a list of matches instead of a dict.
  • get_events() is annotated -> dict[str, str | int]. The base driver (packages/base-driver/lib/basedriver/core.ts and driver.ts) stores every value as a list. commands is a list of {cmd, startTime, endTime} dicts, and every other key is a list of millisecond timestamps. The existing unit test already uses {'appium:funEvent': [12347]}.

As a result, mypy rejects correct code. For example, score: float = driver.get_images_similarity(a, b)['score'] fails with expression has type "bytes | dict[Any, Any]", and for c in driver.get_events()['commands'] fails with Item "int" of "str | int" has no attribute "__iter__".

The docstrings also describe visualization as bytes, do not mention multiple or the list result, and do not say that event values are lists.

Fix

  • Annotate find_image_occurrence() as -> dict[str, Any] | list[dict[str, Any]].
  • Annotate get_images_similarity() and get_events() as -> dict[str, Any].
  • In the docstrings, describe visualization as a base64 str, document the multiple option, the score entry and the list result, and say that every event value is a list.
  • In match_images_features(), which already returns dict[str, Any], correct the same visualization (bytes) entry and describe points1 and points2 as lists, as in MatchingResult in packages/opencv/lib/types.ts. Also fix a visualize (boo) typo.

There is no runtime change.

Behaviour change: type-checker results change. The examples above now pass. Code that indexes the result of find_image_occurrence() directly now needs to narrow it first, for example with isinstance(result, dict). On master the same code was already rejected, because bytes cannot be indexed with a string.

Tests

  • I checked a small script with mypy. On master it reports three errors: the score assignment, iterating events['commands'], and indexing a command dict. With this change it reports none, and find_image_occurrence(..., multiple=True) is revealed as dict[str, Any] | list[dict[str, Any]].
  • ruff check, ruff format --check, mypy appium and the full unit test suite pass.

🤖 Generated with Claude Code

find_image_occurrence and get_images_similarity were annotated as
returning dict[str, bytes | dict], and get_events as returning
dict[str, str | int]. None of these match what the server sends.

The images plugin (packages/images-plugin/lib/compare.ts in
appium/appium) returns the score as a number and the visualization as
a base64 string, and with the multiple option find_image_occurrence
returns a list of matches instead of a single dict. The base driver
event log (packages/base-driver/lib/basedriver/core.ts) stores every
event as a list: `commands` holds dicts with cmd, startTime and
endTime, and each other key holds a list of millisecond timestamps.

Annotate find_image_occurrence as
dict[str, Any] | list[dict[str, Any]], get_images_similarity as
dict[str, Any] and get_events as dict[str, Any]. The docstrings now
describe visualization as a str, document the multiple option, the
score entry and the list result, and describe the event lists. The
same visualization and points types are corrected in
match_images_features, and a `boo` typo is fixed. There is no runtime
change.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant