Unable to validate FeatureCollection

[saurfangg]

Hello,

I was trying to see whether the is_valid would work for feature collection. I tested using the Example GeoJSON Feature Collection.

import geojson
test_fc = { "type": "FeatureCollection", "features": [ { "type": "Feature", "geometry": {"type": "Point", "coordinates": [102.0, 0.5]}, "properties": {"prop0": "value0"} }, { "type": "Feature", "geometry": { "type": "LineString", "coordinates": [ [102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0] ] }, "properties": { "prop0": "value0", "prop1": 0.0 } }, { "type": "Feature", "geometry": { "type": "Polygon", "coordinates": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ] }, "properties": { "prop0": "value0", "prop1": {"this": "that"} } } ] }

geojson.is_valid(geojson.FeatureCollection(test_fc)
('message': '', 'valid': 'yes'}

However it seems that it returns valid for just about anything

test_fc = 'BeepBoop'
geojson.is_valid(geojson.FeatureCollection(test_fc)
('message': '', 'valid': 'yes'}

Is there a different way to go about this or is FeatureCollection validation not supported at this moment?

[frewsxcv]

That looks to be a bug. This is the problematic line. This should probably return an error.

[frewsxcv]

Thanks for the bug report! 🐛

[saurfangg]

@frewsxcv No problem, glad to help! Does this mean however that all validations are in effect not working as an empty string is returned every time?

Also, would it be a better idea to use a boolean instead of a string for 'valid' ?

[frewsxcv]

Does this mean however that all validations are in effect not working as an empty string is returned every time?

From what I can tell, we return an empty string if the validation is successful or if we've encountered an unknown type. I'm not a huge fan of the current format personally, so if you think you know a better format for this, let me know. It might make sense to have a new class ValidationResult instead of just using a dict.

Also, would it be a better idea to use a boolean instead of a string for 'valid' ?

Yes, a boolean for that value would definitely be an improvement.

[lafrech]

While we're at it, maybe is_valid() should return a boolean, as this is what the name suggests, and validate() or something else could return a structure with validation message. In this structure, I also think that a boolean is better than a 'yes'/'no' string (which looks like an anti-pattern).

Another pattern (inspired from Marshmallow) would be to have a validate() method that returns a structure with validation errors as message, and this method would have an optional parameter strict. When strict is True, validate() raises a validation exception (which may hold error messages as well). strict should default to True IMHO.

[zoranbosnjak]

I have created a pull request that fixes this bug.
#98
The obj.validate() method and ValidationResult as mentioned above would be even cleaner solution, but it's easy to build them on top of this pull request.