Feat/validate json #69
Conversation
geojson_pydantic/geometries.py
Outdated
| def validate(cls, value): | ||
| try: | ||
| return cls(**json.loads(value)) | ||
| except Exception: |
There was a problem hiding this comment.
Is the goal of this except block to catch errors on json.loads? If so I think the except block should be more narrowly scoped to TypeError instead of Exception. As written we risk swallowing validation errors if JSON is parseable by json.loads. In general I think this method should separate parsing of value from validation of value by passing into cls.
There was a problem hiding this comment.
As written we risk swallowing validation errors if JSON is parseable by json.loads
Oh yes, you are right, I will update to fix this issue.
| value = json.loads(value) | ||
| except TypeError: | ||
| try: | ||
| return cls(**value.dict()) |
There was a problem hiding this comment.
Whats the reason to check for .dict() attribute here?
There was a problem hiding this comment.
Because we can create a GeometryCollection with Geometry as arguments, which has a List[Geometry] as attribute (Geometry = Union[Point, MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon].)
The validation of the geometries: List[Geometry] attribute is run with an object of type Geometry. Such object cannot be casted into a sublass of _GeometryBase itself, but since it's a subclass to Pydantic BaseModel, we can just call .dict to get the value as a dict.
There was a problem hiding this comment.
And I prefer the EAFP principle instead of LYBL that would had required a check of instance of BaseModel to validate such object.
There was a problem hiding this comment.
Cool that makes sense to me, thanks for the explanation!
|
@vincentsarago this LGTM |
|
@Vikka I don't seems to be able to push to your branch 🤷♂️ |
You should be able to do it now. |
|
🙏 thanks for the PR and nice explanation @Vikka |
What I am changing
How I did it
_GeometryBasebase class and loads the value.How you can test it
<GeometryType>.validate(<json geojson formatted>), e.g.:Point.validate('{"type": "Point", "coordinates": (1, 2)}')return a geojson_pydantic.Point object.Related Issues