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

feat: Geohash class #317

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

feat: Geohash class #317

wants to merge 17 commits into from

Conversation

WenJJ2000
Copy link

@WenJJ2000 WenJJ2000 commented Mar 4, 2024

Hi, we (@WenJJ2000 @karlsb @burcukilic @muchembledMartin @LinusWallin) are a group of university students who would like to be assigned this issue. We have worked on the issue and here is our pull request.

Adds Geohash class and Geohash tests which has the following functionalities :

  • generating an envelope of the geohash cell
  • converting a point to the geohash of given length
  • gets the longest geohash that contains the envelope
  • and generates up to four geohashes that completely cover given envelope

Solves issue #213

LinusWallin and others added 8 commits February 22, 2024 11:38
COMMIT.md contains the structure for how we should add tags to our
commits to make them easier to understand.
docs: Added md file that describes the tags for our commits
…ructure

Revert "docs: Added md file that describes the tags for our commits"
commit b29091a
Merge: 9b64610 9aaaf4c
Author: Wen Jun Jie <132931864+WenJJ2000@users.noreply.github.com>
Date:   Mon Mar 4 10:09:16 2024 +0100

    Merge pull request Esri#55 from DD2480-Group-3/54-coveringGeohash

    54 covering geohash

commit 9aaaf4c
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 10:00:46 2024 +0100

    Squashed commit of the following:

    commit 279212f
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Mon Mar 4 09:46:08 2024 +0100

        fix: solves bug where precision is allowed to be too high

        Limits the precision in containingGeohash function to 6 from 24
        since the toGeohash function has been updated to only work for
        highest precision 6.

    commit 2aa650f
    Merge: 263f8e2 dcdb58d
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Mon Mar 4 09:45:04 2024 +0100

        Merge branch '213-geometry-to-geohash' of github.com:DD2480-Group-3/geometry-api-java into 54-coveringGeohash

    commit 263f8e2
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:47:34 2024 +0100

        refactor: removed missed empty line

    commit 72b84f4
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:47:05 2024 +0100

        refactor: removed empty line

    commit 6fef312
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:45:34 2024 +0100

        refactor: removed empty lines

    commit 1287b2c
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:39:49 2024 +0100

        test: changed envelope to encompass 4 different parts of the geo grid

        The envelope was previously assigned wrong max x and y values, which
        resulted in errors as the coveringGeohash function didn't return
        the expected amount of geo hashes.

    commit 69ca9c0
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:30:38 2024 +0100

        fix: solves bug which resulted in wrong geohashes

    commit 2a8d131
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:24:32 2024 +0100

        test: updated tests to match the changes of the function in last commit

    commit bb45871
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:24:02 2024 +0100

        refactor: made the return string array dynamic

    commit 708542e
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:42:15 2024 +0100

        refactor: removes indentation error caused by merge

    commit a216691
    Merge: 4420765 62f7d5f
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:41:36 2024 +0100

        Merge branch '213-geometry-to-geohash' of github.com:DD2480-Group-3/geometry-api-java into 54-coveringGeohash

    commit 4420765
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:37:30 2024 +0100

        test: added test cases for coveringGeohash function

    commit 2c31979
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:15:40 2024 +0100

        refactor: removes unnecessary if statement

        Removes if statement which didn't change the outcome of the program.

    commit e0b04d3
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 17:39:37 2024 +0100

        feat: coveringGeohash funciton added

        Adds funtion which given an envelope returns up to four geohashes
        which cover the envelope.

commit 9b64610
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 09:24:34 2024 +0100

    docs : update doucmentation for helper function and removed commented code

commit dcdb58d
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 07:52:31 2024 +0100

    refactor : Changed toGeoHash function to use bitwise operations instead of strings to improve time complexity

commit 80aa932
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 00:33:36 2024 +0100

    refactor : changing helper functions to private instead of public and removing their tests

commit 62f7d5f
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:48:48 2024 +0100

    Test: added some tests for containingGeohash

commit 7191907
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:47:58 2024 +0100

    Fix: lat and lon were inverted in test for toGeohash

commit b21e5b9
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:45:28 2024 +0100

    Feat: Implemented containingGeohash

commit c30e1e5
Merge: 84ef58e dbe750f
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:44:34 2024 +0100

    Fix: lat and lon were inverted in tooGeoHash

commit 84ef58e
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 11:02:32 2024 +0100

    test: Added some tests for toGeohash Esri#213

commit dbe750f
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sun Mar 3 00:21:25 2024 +0100

    tests : added test for TestToGeoHash

commit e4adae4
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sat Mar 2 23:32:43 2024 +0100

    tests : added test for binaryToBase32 and TestCovertToBinary

commit 42b5287
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sat Mar 2 23:14:06 2024 +0100

    feat : Added toGeoHash function with 2 helper function BinaryToBase32 and converyTobinary

commit 5fd7cb0
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 17:33:31 2024 +0100

    Feat: Implemented geohashToEnvelope Esri#213

commit cac7dfd
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 17:30:08 2024 +0100

    test: added some tests for geohashToEnvelope Esri#213

commit 8461a3c
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 16:15:02 2024 +0100

    Test: Created a test file for geohash Esri#213

commit af667b6
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 14:45:05 2024 +0100

    Feat: Created Geohash class and its skeletton Esri#213
* docs: Added md file that describes the tags for our commits

COMMIT.md contains the structure for how we should add tags to our
commits to make them easier to understand.

* Revert "docs: Added md file that describes the tags for our commits"

* Squashed commit of the following:

commit b29091a
Merge: 9b64610 9aaaf4c
Author: Wen Jun Jie <132931864+WenJJ2000@users.noreply.github.com>
Date:   Mon Mar 4 10:09:16 2024 +0100

    Merge pull request Esri#55 from DD2480-Group-3/54-coveringGeohash

    54 covering geohash

commit 9aaaf4c
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 10:00:46 2024 +0100

    Squashed commit of the following:

    commit 279212f
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Mon Mar 4 09:46:08 2024 +0100

        fix: solves bug where precision is allowed to be too high

        Limits the precision in containingGeohash function to 6 from 24
        since the toGeohash function has been updated to only work for
        highest precision 6.

    commit 2aa650f
    Merge: 263f8e2 dcdb58d
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Mon Mar 4 09:45:04 2024 +0100

        Merge branch '213-geometry-to-geohash' of github.com:DD2480-Group-3/geometry-api-java into 54-coveringGeohash

    commit 263f8e2
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:47:34 2024 +0100

        refactor: removed missed empty line

    commit 72b84f4
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:47:05 2024 +0100

        refactor: removed empty line

    commit 6fef312
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:45:34 2024 +0100

        refactor: removed empty lines

    commit 1287b2c
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:39:49 2024 +0100

        test: changed envelope to encompass 4 different parts of the geo grid

        The envelope was previously assigned wrong max x and y values, which
        resulted in errors as the coveringGeohash function didn't return
        the expected amount of geo hashes.

    commit 69ca9c0
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:30:38 2024 +0100

        fix: solves bug which resulted in wrong geohashes

    commit 2a8d131
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:24:32 2024 +0100

        test: updated tests to match the changes of the function in last commit

    commit bb45871
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 19:24:02 2024 +0100

        refactor: made the return string array dynamic

    commit 708542e
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:42:15 2024 +0100

        refactor: removes indentation error caused by merge

    commit a216691
    Merge: 4420765 62f7d5f
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:41:36 2024 +0100

        Merge branch '213-geometry-to-geohash' of github.com:DD2480-Group-3/geometry-api-java into 54-coveringGeohash

    commit 4420765
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:37:30 2024 +0100

        test: added test cases for coveringGeohash function

    commit 2c31979
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 18:15:40 2024 +0100

        refactor: removes unnecessary if statement

        Removes if statement which didn't change the outcome of the program.

    commit e0b04d3
    Author: Linus Wallin <linuswallin@live.se>
    Date:   Sun Mar 3 17:39:37 2024 +0100

        feat: coveringGeohash funciton added

        Adds funtion which given an envelope returns up to four geohashes
        which cover the envelope.

commit 9b64610
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 09:24:34 2024 +0100

    docs : update doucmentation for helper function and removed commented code

commit dcdb58d
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 07:52:31 2024 +0100

    refactor : Changed toGeoHash function to use bitwise operations instead of strings to improve time complexity

commit 80aa932
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Mon Mar 4 00:33:36 2024 +0100

    refactor : changing helper functions to private instead of public and removing their tests

commit 62f7d5f
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:48:48 2024 +0100

    Test: added some tests for containingGeohash

commit 7191907
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:47:58 2024 +0100

    Fix: lat and lon were inverted in test for toGeohash

commit b21e5b9
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:45:28 2024 +0100

    Feat: Implemented containingGeohash

commit c30e1e5
Merge: 84ef58e dbe750f
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 16:44:34 2024 +0100

    Fix: lat and lon were inverted in tooGeoHash

commit 84ef58e
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sun Mar 3 11:02:32 2024 +0100

    test: Added some tests for toGeohash Esri#213

commit dbe750f
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sun Mar 3 00:21:25 2024 +0100

    tests : added test for TestToGeoHash

commit e4adae4
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sat Mar 2 23:32:43 2024 +0100

    tests : added test for binaryToBase32 and TestCovertToBinary

commit 42b5287
Author: wenjj2000 <wenjj2000@gmail.com>
Date:   Sat Mar 2 23:14:06 2024 +0100

    feat : Added toGeoHash function with 2 helper function BinaryToBase32 and converyTobinary

commit 5fd7cb0
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 17:33:31 2024 +0100

    Feat: Implemented geohashToEnvelope Esri#213

commit cac7dfd
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 17:30:08 2024 +0100

    test: added some tests for geohashToEnvelope Esri#213

commit 8461a3c
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 16:15:02 2024 +0100

    Test: Created a test file for geohash Esri#213

commit af667b6
Author: --replace-all <muchembled.martin@gmail.com>
Date:   Sat Mar 2 14:45:05 2024 +0100

    Feat: Created Geohash class and its skeletton Esri#213

* Docs : added GeoHash patch file

---------

Co-authored-by: karlsb <36365664+karlsb@users.noreply.github.com>
Co-authored-by: wenjj2000 <wenjj2000@gmail.com>
@randallwhitman randallwhitman requested a review from stolstov March 4, 2024 16:49
@stolstov
Copy link
Member

stolstov commented Mar 4, 2024

Thank you for the contribution.

Could you format it using tabs, similar to other files in the repo?

I've done this:

		String geohash1 = "9qh9mzv6sgtkwz34";
		Envelope2D env1 = Geohash.geohashToEnvelope(geohash1);

and got

-38.41176850208285
-38.41176850241027
75.14555980573277
75.14555980556906

This is not what I expect to see. I checked with some other implementation and also with this website http://geohash.co/

		Point2D p = new Point2D(61.57057257930285, -43.79239231349915);
		System.out.println();
		for (int i = 0; i < 12; i++)
			System.out.println(Geohash.toGeohash(p, i + 1));

This prints

m
m2
m25
m25y
m25yc
m25yc5

The values seem to be correct, but it throws at trying to do more than that. I would expect at least 20 character supported.

@WenJJ2000
Copy link
Author

WenJJ2000 commented Mar 6, 2024

Hi @randallwhitman @stolstov ,the bugs should all have been fixed!

Fixes the first bug mentioned in the previous PR
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.

5 participants