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

Points aren't properly parsed into H3 cells #25

Closed
RaczeQ opened this issue Jul 27, 2023 · 4 comments
Closed

Points aren't properly parsed into H3 cells #25

RaczeQ opened this issue Jul 27, 2023 · 4 comments

Comments

@RaczeQ
Copy link

RaczeQ commented Jul 27, 2023

I was testing the library with different geometries, and I found out that points aren't properly matched to their H3 cells. I could filter out the points and parse them using official H3 bindings, but maybe there is an option to properly parse them in this library in a full package. I haven't tested the linestrings / multilinestrings and geometry collections yet.

from h3ronpy.pandas.vector import geometry_to_cells
from shapely.geometry import Point
import h3

# Manhattan Central Park
point = Point(-73.9575, 40.7938)

h3.int_to_str(geometry_to_cells(point, 8)[0])
# 8875588a83fffff - random cell near Null Island (0, 0)

h3.latlng_to_cell(point.y, point.x, 8)
# 882a1008d7fffff - proper cell
@nmandery
Copy link
Owner

nmandery commented Jul 28, 2023

Interesting - thank you for this report.

I just played around with this and was able to reproduce the issue with h3o directly, without h3ronpy being involved.

    #[test]
    fn central_park_point() {
        // Manhattan Central Park
        let pt = geo_types::Point::new(-73.9575, 40.7938);
        let cells: Vec<_> = h3o::geom::Geometry::from_degrees(geo_types::Geometry::Point(pt))
            .unwrap()
            .to_cells(Resolution::Eight)
            .collect();
        dbg!(LatLng::from(cells[0]));

        //  Using h3-py v3.7.x:
        //
        // $ python
        // Python 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] on linux
        // Type "help", "copyright", "credits" or "license" for more information.
        // >>> import h3.api.numpy_int as h3
        // >>> h3.geo_to_h3(40.7938, -73.9575, 8)
        // 613229523021856767

        assert_eq!(cells[0], CellIndex::try_from(613229523021856767).unwrap())
    }

output:

  Running unittests src/lib.rs (target/debug/deps/h3arrow-23df59f47b072b79)
[src/array/from_geo.rs:359] LatLng::from(cells[0]) = LatLng {
    lat_rad: 0.012440379887428356,
    lat_deg: 0.712781263089079,
    lng_rad: -0.022497083925138273,
    lng_deg: -1.288987960262031,
}


Left:  58-530425017777777 (8875588a83fffff)
Right: 21-020021537777777 (882a1008d7fffff)

thread 'array::from_geo::tests::central_park_point' panicked at 'assertion failed: `(left == right)`
  left: `58-530425017777777 (8875588a83fffff)`,
 right: `21-020021537777777 (882a1008d7fffff)`', src/array/from_geo.rs:360:9

@grim7reaper: Are you aware of any issues like this, or do you see anything I am messing up here?

@grim7reaper
Copy link

Yeah, mistake on my side.

When I changed the API to expose degrees by default (instead of radians, which is what H3 uses internally) I forgot to update a conversion method.

I've just released a version with the fix.

@nmandery
Copy link
Owner

Wow - you are fast ;)

I see you already released a patch version - @RaczeQ I will also release a bugfix-release soon.

nmandery added a commit that referenced this issue Jul 28, 2023
@nmandery
Copy link
Owner

h3ronpy v0.17.4 with the fix is now on PyPI

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

No branches or pull requests

3 participants