feat(LAB-4426): create and export pixel-labeled geospatial projects - #2058
Conversation
RuellePaul
left a comment
RuellePaul
left a comment
There was a problem hiding this comment.
Automated review — LAB-4426 (SDK part)
Reviewed against the ticket's SDK scope (configure pixel mode at creation, export in pixel, disable GeoJSON) and cross-checked with the sibling MRs: export !376, import !463, kili !14307.
Ran pytest tests/unit/services/export tests/unit/adapters/kili_api_gateway tests/integration — 337 passed.
[REQUIRED] pixel_labeling is validated after the project is created — and it can never be used with the copy path
src/kili/presentation/client/project.py:96-113
ProjectUseCases.create_project(...) runs first, and only then is input_type != "GEOSPATIAL" checked. Two consequences, both reproduced against a mocked gateway:
create_project(input_type="IMAGE", json_interface={}, title="t", pixel_labeling=True)→ the project is created on the backend, thenValueErroris raised. The user gets an error suggesting nothing happened, but an orphan project is left behind.create_project(project_id="<some GEOSPATIAL project>", title="copy", pixel_labeling=True)→ always raises, because on the copy pathinput_typeisNone(the input type comes from the copied project), even though the copy is geospatial. Here too the copy is created first and then abandoned.
Suggested fix — validate before any remote call, and handle the copy path explicitly:
if pixel_labeling and project_id is None and input_type != "GEOSPATIAL":
raise ValueError("`pixel_labeling` is only available for `GEOSPATIAL` projects.")...placed at the top of create_project, plus either resolving the copied project's inputType for the project_id branch or documenting that pixel_labeling is not supported when copying.
Related, same block: if the follow-up update_properties_in_project call fails for any reason, the project exists but is silently not in pixel mode. It is still recoverable (the backend only refuses the toggle once assets exist — kili!14307, legacy/mutations.ts), but the error should tell the user the project was created and how to finish configuring it.
[QUESTION] geospatialSettings is now sent/queried unconditionally — what is the minimum supported app version?
src/kili/adapters/kili_api_gateway/project/mappers.py:38— everyupdatePropertiesInProjectcall now carriesgeospatialSettings(includingnull), for every project type.src/kili/services/export/format/base.py:107— every export now requestsgeospatialSettingson the project, for every input type and every format.
geospatialSettings was added to the backend project schema in July 2025 (LAB-3739). Against an older on-prem/LTS backend, both become GraphQL validation errors that break flows unrelated to this feature (any project update, any export). There is precedent for exactly this concern a few lines away:
# src/kili/adapters/kili_api_gateway/project/operations_mixin.py:102-104
# compliance tags are only available for Kili app > 2.138
if "complianceTags" in data and data["complianceTags"] is None:
del data["complianceTags"]If older backends are in scope, dropping the key when None (same pattern) and gating the export field would keep this change inert for everyone else.
[QUESTION] Which layer's dimensions should be used on a multi-layer asset?
src/kili/services/export/format/pixel_labeling.py:23-30
get_asset_pixel_dimensions returns the first layer that has width/height. Per import !463 (_build_layer_geo_metadata), in pixel mode every layer of the asset gets its own width/height from get_raster_size — so a multi-layer asset with layers of different raster sizes will have all its labels scaled by the first layer's size, silently. Is a pixel-mode asset guaranteed single-layer, or are labels always normalized against layer 0?
Minor detail in the same area: the test fixture carries "labelingCRS": "PIXEL" per layer, but the code never reads it — worth either using it as the selector or dropping it from the fixture so it doesn't imply a filter that isn't there.
[QUESTION] Exported normalizedVertices hold unnormalized pixels — deliberate?
src/kili/services/export/format/pixel_labeling.py:37-58
The scaling overwrites boundingPoly[].normalizedVertices, point and polyline in place, so the exported JSON has a key named normalizedVertices containing pixel values. This matches label_export/pixel_labeling.py in export !376 exactly, so it looks intentional and the SDK/platform stay consistent — but it differs from the image export the ticket points to ("same export as image"): for IMAGE, kili_formats keeps normalizedVertices normalized and adds vertices / pointPixels / polylinePixels. Practical consequence: a pixel-project export can no longer be fed back through append_labels as-is. Worth confirming and calling out in the create_project docstring if so.
[SUGGESTION] The missing-dimensions fallback is silent
src/kili/services/export/format/pixel_labeling.py:68-73
When geospatialExportMetadata has no dimensions, convert_to_pixel_coords returns the asset untouched, so that asset's labels stay normalized while every other asset in the same archive is in pixels — with nothing in the output to distinguish them. KiliExporter has self.logger; a warning naming the asset would make this diagnosable.
[SUGGESTION] Pose-estimation points are not scaled
src/kili/services/export/format/pixel_labeling.py:14,37-58
_VERTEX_CONTAINERS covers boundingPoly, polyline, point, vertices, but not the points list of pose-estimation annotations ([{"point": {...}}, ...]), which kili_formats.tool.video.scale_normalized_vertices_image_video_annotation does handle. If such a job ever exists in a pixel project, those coordinates stay normalized. Cheap to cover — and would need the same fix in export !376.
[SUGGESTION] Test parity with the twin module in export !376
tests/unit/services/export/test_pixel_labeling.py
The export service's test/unit/test_pixel_labeling.py covers two cases this file dropped: test_convert_point_and_polyline_to_pixels and is_pixel_labeling_project({"geospatialSettings": None}). As it stands, the SDK's point / polyline / vertices branches and the None settings branch are untested. Also uncovered: create_project(pixel_labeling=True) (ordering + mutation payload — the bug above would have been caught) and the GeoJSON refusal at src/kili/services/export/format/geojson/__init__.py:50.
[PRAISE] Faithful twin of the platform export
src/kili/services/export/format/pixel_labeling.py
Keeping this module a line-for-line mirror of label_export/pixel_labeling.py (export !376), down to the refusal message, is the right call — SDK and platform exports cannot drift. Two other things checked out: coco / yolo / pascal_voc already reject GEOSPATIAL, so kili/raw and geojson really are the only reachable formats and both are handled; and the field sets in get_fields_to_fetch are mutually exclusive, so the non-idempotent in-place scaling can't be applied twice to the same asset.
Summary: 1 required, 3 questions, 3 suggestions, 1 praise. The export path is solid and consistent with the platform; the one thing to fix before merge is the create_project validation ordering, which today silently creates orphan projects and makes pixel_labeling unusable when copying a project. The geospatialSettings backward-compatibility question is worth a decision even if the answer is "current versions only".
🤖 Automated review — /auto-review
5a6bb50 to
a2dc981
Compare
6fbeb7d to
3f3c1d4
Compare
Add a pixel_labeling flag to create_project for geospatial projects, where the image is annotated exactly as captured by the sensor and geographic coordinates come from a customer-hosted pixel-to-geo service. It is applied through the project geospatial settings, which update_properties_in_project now exposes, and can only be set while the project has no asset. The Kili export unnormalizes those labels into image pixels, and GeoJSON is refused for them since Kili holds no trustworthy geolocation to export.
…load test pyright rejects passing ProjectDict where a plain dict is expected, and the update_properties_in_project payload assertion has to account for the new geospatialSettings key.
The check ran after createProject, leaving an orphan project behind, and always raised when copying an existing project since input_type is then unset. Pose estimation points and nested job annotations are now unnormalized too, and geospatialSettings is omitted rather than sent as null so update_properties_in_project keeps working against deployments that do not know the field.
Pose estimation is not among the geospatial tools, sub-jobs are limited to classification and transcription, and `vertices` belongs to the annotation API rather than the json response. Only boundingPoly, polyline and point remain. Spells out the accepted geospatial_settings keys and their values.
The unnormalization overwrote `normalizedVertices` with pixel values, so a consumer reading that key got thousands where it expected fractions. Mirror what an image project does instead: leave `normalizedVertices` normalized and add `vertices`, `pointPixels` and `polylinePixels`. Keeps the helper in sync with the export service.
Pass pixelLabeling through the create mutation, which now carries the field, instead of creating the project and then updating its geospatial settings. update_properties_in_project loses the geospatial_settings argument it had gained: it was only ever there to reach labelingCRSCode, and reaching it is exactly what must not be possible once the project exists.
The dimensions were taken from the first layer that carried any. An asset can hold a reference image of another size beside the one being labeled, and unnormalizing against that would move every annotation without a word. Take the first layer, which the labeling grid is built from, and export the fractions untouched when it has no dimensions -- the existing degraded path -- rather than sliding to a layer whose grid the annotations never used. Keeps the helper in sync with the export service.
3f3c1d4 to
9f61118
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
|
||
| return ProjectId(new_project_id) | ||
|
|
||
| def _copy_project( |
There was a problem hiding this comment.
[BLOCKER] Copying a pixel-labeled project silently produces a non-pixel one
create_project routes project_id is not None to _copy_project, which has no
pixel_labeling parameter and never reads the setting from the source project. So
copying a pixel-labeled geospatial project yields a normal geospatial project — and
that can't be corrected afterwards, since the mode is creation-only by design.
It fails silently in the other direction too. The client guard is:
if pixel_labeling and input_type is not None and input_type != "GEOSPATIAL":
On the copy path input_type is None, so create_project(project_id=..., pixel_labeling=True)
passes validation and is then discarded without a word.
Tagged BLOCKER rather than REQUIRED because the resulting project is in a wrong state
that no API call can repair — the user's only recourse is to delete and recreate, losing
any work already done in it.
Suggested fix: read geospatialSettings.labelingCRSCode from the copied project and
forward it, or raise explicitly when pixel_labeling is passed alongside project_id.
| @@ -0,0 +1,101 @@ | |||
| """Export of geospatial projects labeled in the image's own sensor pixel grid. | |||
There was a problem hiding this comment.
[REQUIRED] This belongs in kili-formats, not vendored here
The SDK already depends on kili-formats for every other export format, and
src/kili_formats/kili.py owns per-input-type coordinate conversion
(_scale_label_vertices, convert_to_pixel_coords) — whose GEOSPATIAL branch is currently
an early return. That branch is where pixel labeling belongs.
Instead this is 101 lines copied verbatim from export/src/label_export/pixel_labeling.py,
kept aligned by hand. Comment 3 below is the cost made concrete: the same boundingPoly
defect exists in both copies and has to be fixed in both, and until it is, the SDK export and
the app export produce different coordinates for the same label.
Same point is on export !376; whichever repo moves first, they should land together.
The release overhead is not a reason to defer: bump the version in pyproject.toml, tag the
merge commit, create a release note (README "Release"), then bump the pin here. kili-formats
is at 1.4.0 and this is an additive change to an existing branch, so it is a minor bump with no
migration for existing callers.
| return {**vertex, "x": vertex["x"] * width, "y": vertex["y"] * height} | ||
|
|
||
|
|
||
| def _scale_annotation(annotation: dict, width: int, height: int) -> None: |
There was a problem hiding this comment.
[BLOCKER] Semantic annotations crash the export — same bug as export !376
This file is a verbatim copy of export/src/label_export/pixel_labeling.py, including the
defect: _scale_annotation assumes the flat, image-style boundingPoly
([ {normalizedVertices: [...]}, ... ]), but geospatial semantic stores one annotation whose
boundingPoly is hierarchical — a list of polygon groups, each a list of rings:
boundingPoly = [ [ {normalizedVertices: [...]}, ... ], ... ]
Each element is then a list, so {**norm_vertices, ...} raises
TypeError: 'list' object is not a mapping before the subscript is reached. Boxes and
polygons are flat, which is why only semantic fails.
kili-formats already distinguishes the two — format/geojson/segmentation.py:_is_hierarchical_format.
No change is needed in kili: UseCasesLabelJsonResponse.ts already emits the hierarchical
shape for geospatial and only splits it into flat per-mid annotations for non-geospatial input
types. The defect is entirely on the consumer side.
Full reasoning and the suggested fix are on export !376
(src/label_export/pixel_labeling.py:45). Both copies must change together, or the SDK export
and the app export will produce different coordinates for the same label.
Auto Review SummaryVerdict: Needs changes
The |
3 participants
Part of LAB-4426 — pixel labeling of classified imagery.
What it adds
create_project(..., pixel_labeling=True), forGEOSPATIALprojects only. It defines the reference frame the labels are stored in, so it is settable only at creation — there is no argument anywhere to change it afterwards, and the backend refuses any switch. It is passed through the creation mutation itself, in a single call.verticesnext toboundingPoly[].normalizedVertices, andpointPixels/polylinePixelsnext topoint/polyline. The normalized values are left untouched, so a consumer reading them keeps reading fractions.The dimensions used to unnormalize come from
geospatialExportMetadata, recorded at import. An asset that has none keeps only its fractions, and says so in the logs.Requires the
pixelLabelingfield onCreateProjectData, added in kili!14307 — that MR has to be deployed first.pixel_labelingis simply omitted from the mutation when false, so nothing changes for existing callers.create_projectvalidatespixel_labelingagainst the input type before any remote call, so an invalid combination cannot leave an orphan project behind.The export helper is kept byte-identical to the export service's (kili-technology/services/export!376) so the two cannot drift.