Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NZTM2000 TMS fails test if BBox is removed #103

Closed
dchirst opened this issue May 2, 2023 · 9 comments
Closed

NZTM2000 TMS fails test if BBox is removed #103

dchirst opened this issue May 2, 2023 · 9 comments

Comments

@dchirst
Copy link
Contributor

dchirst commented May 2, 2023

If you remove the "boundingBox" section from the NZTM2000.json TMS definition, the following test fails

def test_InvertedLatLonGrids():
        """Check Inverted LatLon grids."""
        tms = morecantile.tms.get("NZTM2000")
        bound = tms.xy_bounds(morecantile.Tile(1, 2, 0))
        assert bound == (1293760.0, 3118720.0, 3587520.0, 5412480.0)
>       assert tms.xy_bbox == (274000.0, 3087000.0, 3327000.0, 7173000.0)
E       assert BoundingBox(left=-1000000.0, bottom=824960.0, right=3587520.0, top=10000000.0) == (274000.0, 3087000.0, 3327000.0, 7173000.0)
E         At index 0 diff: -1000000.0 != 274000.0
E         Full diff:
E         - (274000.0, 3087000.0, 3327000.0, 7173000.0)
E         + BoundingBox(left=-1000000.0, bottom=824960.0, right=3587520.0, top=10000000.0)

This is important because the boundingBox should be able to be calculated without an explicit definition. In TMS 2.0, the boundingBox property is optional, so we can't rely upon the explicit definition.

Steps to reproduce:

  1. Clone main branch
  2. Remove the boundingBox property from the top of NZTM2000.json
  3. Run pytest

Is there something basic about TMS definitions that I'm missing? And is this an issue better suited for the definition repo at LINZ? Thanks!

@vincentsarago
Copy link
Member

It's funny because I've spend some time couple weeks ago trying to figure how the NZTM2000was made and was intrigued by the bbox too.

I guess the best will be to ask directly LINZ people in their repo.

@vincentsarago
Copy link
Member

Note:

Its value is somehow informative. I'm not sure what the TMS 2.0 tell about it

@dchirst
Copy link
Contributor Author

dchirst commented May 2, 2023

Good to know that you've seen something weird too! I will make an issue with the LINZ people and see what they say.

@vincentsarago
Copy link
Member

@dchirst any update? I see that @blacha answered in the Linz issue. I think for now we should add back the wrong bounding box in the TMS 2.0 and we can remove/change it in the future.

@vincentsarago
Copy link
Member

☝️ changed my mind,

I think we should modify https://github.com/developmentseed/morecantile/blob/main/morecantile/models.py#L656-L699 and get the bbox from the first matrix!

@dchirst
Copy link
Contributor Author

dchirst commented May 5, 2023

Hi Vincent! Thank you for this, I think it's the right call. I'll take it and try and round out 4.0 release next week.

@blacha
Copy link
Contributor

blacha commented May 5, 2023

I wonder if another option here is just to drop this tile matrix? I am unsure how many (if anyone?) actually uses it outside of LINZ, Who have deprecated it for internal use cases.

Another fun example for boundingBox math, is the first matrix bounding box does not have to be the same as the second with https://github.com/developmentseed/morecantile/blob/main/morecantile/data/LINZAntarticaMapTilegrid.json#L8 the first two Z's are both 1x1 grids but z1 has a much smaller bounding box. However it is somewhere in our backlog to move this tile grid into a LinzAntarticaQuad

I haven't seen any examples of this in reverse, but there is nothing stopping anyone from making a tileMatrix where z0 is much smaller of a boundingBox than z1 (or even a random z).

@vincentsarago
Copy link
Member

I wonder if another option here is just to drop this tile matrix?

Fine by me as well 🙏

@dchirst
Copy link
Contributor Author

dchirst commented May 6, 2023

Cool! I'll action this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants