Repository navigation
Conversation
| height: Optional[int] | ||
|
|
||
| ConvertTypes = Literal['image/webp', 'image/jpeg', 'image/png', "image/avif", "image/jxl", "*/*"] | ||
| ConvertTypes = Literal['image/webp', 'image/jpeg', 'image/jpg', 'image/png', "image/avif", "image/jxl", "*/*"] |
There was a problem hiding this comment.
We also need a type hint in the API to inform the user that they may use the enum from _format.py
There was a problem hiding this comment.
Fixed in 175af41 — and it was worse than a missing hint: the constants did not typecheck at all.
JXL = "image/jxl" was inferred as str, so convert(type=Format.JXL) failed against the ConvertTypes literal union:
Argument "type" to "convert" of "Source" has incompatible type "str";
expected "Literal['image/webp', ..., 'image/jxl', '*/*']"
The constants were exactly what you could not pass. Each is now annotated Final[Literal[...]], which narrows it to its own literal type and makes it a member of ConvertTypes, so no change to _typed.py is needed.
Used # type: comments with a TYPE_CHECKING-guarded import rather than inline annotations, since setup.py still classifies 2.7 and this module is imported at runtime by __init__.py. Verified with mypy: all three call sites (single, list, bare literal) now pass; python -c "import tinify" still works.
Worth noting the repo's own mypy --check tinify would not have caught this — it checks the package, not call sites that consume these hints.
|
Heads-up: the three Mypy jobs now fail on This is argument parsing — it happens before mypy opens a single source file, so no code change can influence it.
Verified with a throwaway branch at I left the workflow alone on purpose. Changing it to That reproduces identically on Unit tests pass everywhere: 90 passed, 1 skipped locally and across the CI matrix. One thing worth noting about the defect this PR fixes: |
Adds a Format namespace of media-type string constants so the types accepted by convert() are discoverable via autocomplete, matching the other Tinify client libraries. Also adds the missing image/jpg to ConvertTypes. The API accepts it, but the type hints did not, so passing it failed type checking.
The constants were inferred as plain str, so passing Format.JXL to convert() failed type checking against the ConvertTypes literal union -- the constants were exactly what you could not pass. Annotating each as Final[Literal[...]] via type comments narrows them to their own literal type. Type comments plus a TYPE_CHECKING-guarded import keep the module importable on the Python 2.7 the package still claims. Also clarifies that JPG is not an alias of JPEG, and annotates __init__.
175af41 to
d7d2125
Compare
image/jpg and image/jpeg are different strings, but the API normalises image/jpg to image/jpeg before conversion, so the two constants select the same format and produce identical output. The previous wording said JPG was "not an alias" for JPEG. That answered a question about constant identity while implying the two produce different results, which is the more misleading of the two readings.
Adds a
Formatnamespace of media-type constants so the types accepted byconvertare discoverable via autocomplete — including JPEG XL(
image/jxl), which the API already accepts but no client surfaced.image/jxlis already in the API allowlist (apps/api/src/schemas/optimizations.tsand
apps/opt-api/src/constants/content-types.ts), so this is a client-sidediscoverability change only. There is no API-side work outstanding.
Members
WEBP,PNG,JPEG,JPG,AVIF,JXL,ANY(*/*) — mirroring theAPI allowlist exactly.
Why string constants and not an enum
The JSON body has to carry the media type verbatim. A language-level enum
serializes as its name or ordinal, not
image/jxl. String constants also keepthis fully backward compatible: callers passing raw strings are unaffected.
Also fixed here
ConvertTypeswas missingimage/jpg, which the API does accept — passing itfailed type checking. Added.
Verification
Full suite green locally:
pytest test/unit→ 90 passed, 1 pre-existing skip, including 4 newtinify_format_test.pycases.Part of a coordinated change across all six Tinify client libraries, so the
same
FormatAPI is available in each.