Only do geometries.geoms when the attribute is present.#520
Only do geometries.geoms when the attribute is present.#520EmileSonneveld wants to merge 6 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| def features_ids_from_index(geometries): | ||
| return ["feature_%d" % i for i in range(len(geometries.geoms))] | ||
| geoms = geometries.geoms if hasattr(geometries, "geoms") else geometries | ||
| return ["feature_%d" % i for i in range(len(geoms))] |
There was a problem hiding this comment.
features_ids_from_index is already a bit obscure because of lack of type annotations. Adding this if here makes it worse I think
I would fully eliminate features_ids_from_index and just do the list comprehension inline in the if-else below. Something like
if isinstance(self._regions, GeometryCollection):
...
feature_ids = ["feature_%d" % i for i in range(len(self._regions))]
elif isinstance(self._regions, DriverVectorCube):
...
feature_ids = (list(feature_ids) if feature_ids is not None
else ["feature_%d" % i for i in range(self._regions.geometry_count())])
else:
raise ValueError(self._regions)(Note: GeometryCollection is something we should get rid of, so this would prepare more cleanly for that)
There was a problem hiding this comment.
I used ai first. Could have been better. I used your suggestion now. That works fine
| if not isinstance(regions, (GeometryCollection, DriverVectorCube)): | ||
| # TODO: raise exception instead of warning? | ||
| _log.warning( | ||
| raise ValueError( |
There was a problem hiding this comment.
@soxofaan I did not find this warning in kibana. Safe to convert it to Exception?
2 participants
#519
@copilot review