Open saurfangg opened 8 years ago
That looks to be a bug. This is the problematic line. This should probably return an error.
Thanks for the bug report! 🐛
@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' ?
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.
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.
I have created a pull request that fixes this bug.
https://github.com/frewsxcv/python-geojson/pull/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.
Hello,
I was trying to see whether the is_valid would work for feature collection. I tested using the Example GeoJSON Feature Collection.
However it seems that it returns valid for just about anything
Is there a different way to go about this or is FeatureCollection validation not supported at this moment?