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

Add new failing test case for a bug in tiles(), update requirements.txt #152

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rohits2
Copy link
Contributor

@rohits2 rohits2 commented Dec 1, 2024

Why?

tiles() describes itself as:

Get the tiles overlapped by a geographic bounding box

    Parameters
    ----------
    west, south, east, north : sequence of float
        Bounding values in decimal degrees.
    zooms : int or sequence of int
        One or more zoom levels.
    truncate : bool, optional
        Whether or not to truncate inputs to web mercator limits.

    Yields
    ------
    Tile

    Notes
    -----
    A small epsilon is used on the south and east parameters so that this
    function yields exactly one tile when given the bounds of that same tile.

This is not how it works. If the bounding box stretches across a pole, tiles() will not return anything.

What?

This change fixes the bug and adds a few new tests to check.

These edge cases were previously not checked by ANY tests. The new function description is this:

Get the tiles overlapped by a geographic bounding box.

    NOTE: Web Mercator tiles cannot latitudes beyond ±85.051129.  
    This region is not represented in the returned tile set even if you request it.

    Parameters
    ----------
    west, south, east, north : sequence of float
        Bounding values in decimal degrees.
    zooms : int or sequence of int
        One or more zoom levels.
    truncate : bool, optional
        Whether or not to truncate inputs to web mercator limits.

    Yields
    ------
    Tile

    Notes
    -----
    A small epsilon is used on the south and east parameters so that this
    function yields exactly one tile when given the bounds of that same tile.

How?

I have extended the tile-splitting algorithm to handle more splits, so that it can handle a situation where lnglat bounds straddle the "corners" of the globe. The tl;dr:

    # Create our bbox.
    bboxes = [(west, south, east, north)]
    # If the bbox straddles a pole (north-south), split it in half.
    new_bboxes = []
    for w, s, e, n in bboxes:
        if s > n:
            new_bboxes += [(w, s, e, 90.0), (w, -90.0, e, n)]
        else:
            new_bboxes += [(w,s,e,n)]
    bboxes = new_bboxes

    # If the bboxes straddle the antimeridian (east-west), split them in half.
    new_bboxes = []
    for w, s, e, n in bboxes:
        if w > e:
            new_bboxes += [(-180.0, s, e, n), (w, s, 180.0, n)]
        else:
            new_bboxes += [(w,s,e,n)]
    bboxes = new_bboxes

requirements.txt

I could not pip install -r requirements.txt in my Python 3.12 environment. To fix that, I've updated the file and included a script to do so automatically if it becomes necessary in the future.

This change might be out-of-scope but is included as part of my working environment.

@rohits2 rohits2 changed the title Add new failing test case Add new failing test case for a bug in tiles() Dec 1, 2024
@rohits2 rohits2 marked this pull request as ready for review December 1, 2024 14:39
@rohits2 rohits2 changed the title Add new failing test case for a bug in tiles() Add new failing test case for a bug in tiles(), update requirements.txt Dec 1, 2024
@rohits2
Copy link
Contributor Author

rohits2 commented Dec 1, 2024

I might be kind of slow to reply - no urgency whatsoever to get this merged in.

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.

1 participant