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

The attribute bpolys of indicators should be of type Feature or MultiPolygon not FeatureCollection #9

Closed
matthiasschaub opened this issue May 12, 2021 · 4 comments · Fixed by #69
Assignees
Labels
comments welcome Indicates that the creator of this PR is open for early review comments
Milestone

Comments

@matthiasschaub
Copy link
Collaborator

matthiasschaub commented May 12, 2021

Should an indicator object only have the geometry as attribute or the whole feature (GeoJSON Feature object) including the geometry as attribute?

Also is bpolys a good variable name or should it be renamed to geometry or feature?

@matthiasschaub matthiasschaub added the comments welcome Indicates that the creator of this PR is open for early review comments label May 18, 2021
@matthiasschaub matthiasschaub self-assigned this May 18, 2021
@matthiasschaub
Copy link
Collaborator Author

matthiasschaub commented May 25, 2021

bpolys is used by ohsome API as parameter. It is defined as follows:

WGS84 coordinates given as a list of coordinate pairs (as for bboxes) or GeoJSON FeatureCollection. The first point has to be the same as the last point and MultiPolygons are only supported in GeoJSON; no default value (one boundary parameter must be defined)

@matthiasschaub
Copy link
Collaborator Author

I propose to use geojson.Feature as type of bpolys. That way OQT is agnostic to the geometry type of the feature (e.g. Polygon vs Multipolygon). Maybe rename bpolys to bpoly to differentiate Feature Collection and Feature (ohsome API vs OQT).

@matthiasschaub
Copy link
Collaborator Author

After internal discussions following solution is proposed:

  • Rename bpolys to bpoly
  • Differentiate bpolys as input parameter from bpolys as attribute of indicator/report classes
    • Input to CLI/API should be a GeoJSON where it does not matter if it is a Feature, FeatureCollection or Geometry
    • Attribute should only contain the geometry -> Type is either Polygon or Multipoligon

@matthiasschaub
Copy link
Collaborator Author

After another internal discussions following solution is proposed:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comments welcome Indicates that the creator of this PR is open for early review comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants