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

Draft of a geometry operations applicator #1112

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Draft of a geometry operations applicator #1112

wants to merge 2 commits into from

Conversation

sgillies
Copy link
Member

@sgillies sgillies commented Jun 3, 2022

Resolves #982

@sgillies sgillies added this to the 1.9 milestone Jun 3, 2022
@sgillies sgillies self-assigned this Jun 3, 2022

from libcpp.unordered_map cimport unordered_map

cdef OGRGeometryH segmentize(OGRGeometryH geom, double max_length):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because OGR_G_Segmentize is an oddball and returns nothing (void).

func_map[1] = <geometry_func>segmentize

cdef geometry_func func_alias(int choice):
return func_map[choice]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't just put OGR_G_Segmentize (for example) in a Python dict. https://stackoverflow.com/a/56708967 was very helpful in showing a solution.


op_map = {
"segmentize": 1
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map to the map of OGR functions.

for geom in geoms:
ogr_geom1 = _geometry.OGRGeomBuilder().build(geom)

for op_name, op_arg in operations:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically it's a per-geometry pipeline implemented in C.

The other obvious design is to add segmentize &c as methods to our Geometry class. But performance won't be great until version 2.0 of fiona when Geometry is an extension class and carries an OGRGeometryH or a zero-copy data structure around with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might not be bad to wait until 2.0 for this as the Geometry class seems to have the potential to significantly impact on the design.

@sgillies
Copy link
Member Author

sgillies commented Jun 3, 2022

@snowman2 I have a draft solution for #982 here. I'm going to remove it from the fiona 1.9a2 milestone since there's still some discussion to be had about the design.

@sgillies sgillies removed this from the 1.9 milestone Dec 13, 2022
Base automatically changed from maint-1.9 to master February 26, 2023 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants