Skip to content

fix(geographic): set default target CRS in Extent.as to the target CRS#2811

Open
MatrixNeoKozak wants to merge 2 commits into
iTowns:masterfrom
MatrixNeoKozak:fix/improvement-1783435762101
Open

fix(geographic): set default target CRS in Extent.as to the target CRS#2811
MatrixNeoKozak wants to merge 2 commits into
iTowns:masterfrom
MatrixNeoKozak:fix/improvement-1783435762101

Conversation

@MatrixNeoKozak

Copy link
Copy Markdown

When using Extent.as(crs) without specifying a target Extent instance, it defaulted to creating an Extent in 'EPSG:4326' instead of using the target crs projection. This changes the default target argument from new Extent('EPSG:4326') to new Extent(crs) so that the newly created Extent uses the correct destination coordinate reference system by default. A corresponding unit test is also added to check this behavior.

@ftoromanoff
ftoromanoff requested a review from Desplandis July 8, 2026 12:20
@ftoromanoff

ftoromanoff commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for this correction and for using iTowns !
I was waiting for the tests to end succesfully to send this asnwer.

The idea is good, I'm just wondering what might happened if the crs entered as parameter is not a 'valid' crs for an Extent, but it's probalby not even a question...

PS : After re-read, I feel like there is no need for your proposition as the target extent crs is always set with the entered 'crs' (if we need need to return a new Extent).
Morevoer I tested your propososal for a new unit test on the version before your modification and it passes... The test don't seems to bring anything new

@MatrixNeoKozak

Copy link
Copy Markdown
Author

Hello!
Thank you for the feedback!

While target.crs = crs is eventually set, using new Extent('EPSG:4326') as the default argument bypassed the constructor validation for geocentric projections. Specifically, calling extent.as('EPSG:4978') (a geocentric projection) would succeed and return an Extent object with crs: 'EPSG:4978', violating the class invariant that Extent cannot represent geocentric projections.

To resolve this:

  1. We updated the default argument to new Extent(crs).
  2. We added an explicit geocentric projection check inside as() to prevent returning geocentric extents even when a custom target is provided.
  3. We added a new unit test should throw an error when converting to a geocentric projection with as() that validates this behavior (and would fail under the old implementation).

The changes have been pushed to the branch.

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.

2 participants