Skip to content

switch to cql2 (continuation of #236)#260

Merged
vincentsarago merged 12 commits into
developmentseed:mainfrom
bkanuka:feature/switch-to-cql2
Jun 11, 2026
Merged

switch to cql2 (continuation of #236)#260
vincentsarago merged 12 commits into
developmentseed:mainfrom
bkanuka:feature/switch-to-cql2

Conversation

@bkanuka

@bkanuka bkanuka commented May 27, 2026

Copy link
Copy Markdown
Contributor

Continuation of #236

Still in draft...

@bkanuka bkanuka changed the title switch to cql2 (rebased continuation of #236) switch to cql2 (continuation of #236) May 27, 2026
@bkanuka

bkanuka commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

@vincentsarago can you explain a little more about the approach to CRS ? I don't know if the following is true:

  • Data can be in any CRS, this is specified in PostGIS and we we just ST_Transform to 4326 at query time
  • OGC Features/GeoJSON responses are always in 4326
  • bbox in the query params are always assumed to be 4326? (or are they assumed to be the source CRS?)
  • Tiles can be requested and returned in any CRS but if not specified, then assume Web Mercator EPSG:3857

Could you clarify a bit? Thanks

@bkanuka bkanuka marked this pull request as ready for review May 29, 2026 23:59
@bkanuka

bkanuka commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Removed buildpg and building the WHERE with cql2-rs. Ready for someone else to get eyes on this!

This is kinda a big PR (sorry). Tests all pass.

@vincentsarago vincentsarago changed the base branch from main to feature/switch-to-cql2 June 4, 2026 12:15
@vincentsarago vincentsarago changed the base branch from feature/switch-to-cql2 to main June 4, 2026 12:15
@vincentsarago

Copy link
Copy Markdown
Member

Hi @bkanuka
I'm trying to pick this up

Data can be in any CRS, this is specified in PostGIS and we we just ST_Transform to 4326 at query time

Yes,

tipg/tipg/collections.py

Lines 501 to 503 in 1d269b3

# Reproject to WGS64 if needed
if geometry_column.srid != 4326:
g = logic.Func("ST_Transform", g, pg_funcs.cast(4326, "int"))

OGC Features/GeoJSON responses are always in 4326

Yes

bbox in the query params are always assumed to be 4326? (or are they assumed to be the source CRS?)

Yes, because we choose not to implement part 2 of the spec #35

Tiles can be requested and returned in any CRS but if not specified, then assume Web Mercator EPSG:3857

a TMS object must be passed to the get_tile method

tipg/tipg/collections.py

Lines 856 to 860 in 1d269b3

async def get_tile(
self,
conn: asyncpg.Connection,
*,
tms: TileMatrixSet,
so there no default.

@bkanuka

bkanuka commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Great. That was my understanding, but reasoning about transformations is hard so there's a chance I fumbled it.

Side note: I'm not a fan of 1000+ line files but didn't want this pr to also include refactoring. Maybe that will come later 😉

Comment thread tipg/dependencies.py
comparison. Re-parsing the JSON form forces every spatial literal into
the SRID-4326 emit path. ``_where`` later wraps these 4326 literals in
``ST_Transform(..., <column_srid>)`` if the target column is in a
different CRS, so the index on the column side is preserved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread tipg/collections.py
Comment on lines +575 to +580
transformer = TransformerFromCRS(
tms_epsg, tile_target_srid, always_xy=True
)
west, south, east, north = transformer.transform_bounds(
west, south, east, north
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to see where we do the transformation from TMS's CRS to the Geometry column's CRS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkanuka could you explain? 🙏

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tile_target_srid is the column CRS. So those lines are where the transformation is done. I added code comments that might clarify. (maybe I'm the one confused though!)

Comment thread tipg/collections.py

@vincentsarago vincentsarago left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkanuka This is excellent,

Thanks for all the docstrings, it really helped for the review 🙏

I just have one comment about the tile bbox reprojection, once we figure this we can merge 🙏

@bkanuka

bkanuka commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

I will get back to everything here next week. Still top of mind to get this merged but human life is limiting my response time ATM.

@vincentsarago vincentsarago merged commit 10fe7e6 into developmentseed:main Jun 11, 2026
6 of 7 checks passed
@vincentsarago vincentsarago mentioned this pull request Jun 11, 2026
@vincentsarago

Copy link
Copy Markdown
Member

thanks a lot @bkanuka 🙏

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.

3 participants