unusual bounds checks in provider tests

[missinglink]

Hi Stephan,

I was writing a new provider today and noticed some code which stumped me, it looks like this, and is present in several provider tests:

expect(result.bounds[0][0]).toBeGreaterThan(result.bounds[0][1]);
expect(result.bounds[1][0]).toBeGreaterThan(result.bounds[1][1]);
expect(result.bounds[0][0]).toBeLessThan(result.bounds[1][0]); // south less than north
expect(result.bounds[0][1]).toBeLessThan(result.bounds[1][1]); // west less than east

the bottom two tests are valid, they compare min/max values of the same dimension to ensure the order is valid.

the issue is with the top two, where they are checking that one dimension is greater than the other, this only makes sense in some part of the world and probably shouldn't be in the unit tests, I got confused when I copied the logic and it failed.

let me know what you think and I can clean it up?

[smeijer]

Yeah, that makes sense. Never thought of it to be honest 😅 (living at 53, 5)

Doesn't the same account for the other coordinates? The world is a sphere, so if you would stand on the north pole, the same applies to all 4 checks, right?

But even though it's not right, it is a way to confirm that stuff works as we expected. Should we just change it to ensure it's a number? Or use a fixed search location that matches these conditions, and add a comment?

[missinglink]

haha, so like everything in geo, it's a little tricky...

I'm assuming that the coordinates are in EPSG:4326? I don't believe it's explicitly documented here but I kinda assumed so.

The tricky parts are about 'wrapped' coordinates and the antimeridian, here's looking at you Fiji!

When you zoom out leaflet and pan left or right you end up with longitudes <-180 or >+180, which isn't really a problem for bounding box checks (although it's a little ugly).

But to represent the bounds of a geometry which crosses the antimeridian it's either going to to be lon (179.00,181.00) or (179.00,-179.00) and in the latter case it's going to fail these checks.

Note: flipping the min/max will result in an area that covers most of the world rather than a small slice near the 180th meridian, which we don't want to do ;)

If we want to disallow 'wrapped' coordinates completely then it becomes impossible to represent the bounds with a single box, since the geometry would have to get cut on the antimeridian!

A more robust solution, like you mentioned is to convert the planar coordinates into spherical coordinates and check their relative positions are in the expected (east/west) hemisphere, I'm sure there's a module somewhere which does this if we wanted to use it.

haha, I wouldn't worry about it too much, I think for the most part we just trust the geocoders to handle this stuff internally and say that bounding boxes at the antimeridian are uncommon enough to not worry about.

those two checks "south less than north" and "west less than east" are good to leave in to catch silly configuration errors.

[edit] actually it looks like Pelias doesn't handle this well, I guess I need to open an issue for that 😆

[edit again] we should totally not allow wrapping the latitude, it's a nightmare to deal with (since the lon also implicitly flips around) and without doubt indicates an error.

[smeijer]

Thanks for the explanation. I had no idea, but it makes sense now. Please feel free to submit the fix that you feel is best.

I've also added you as a collaborator on the project, so you can now make your branches directly on the repo rather than your fork if you want.

[missinglink]

ok cool, I'll put in a PR after those other two land because they would touch the same code.