rewrite of validation mechanism #98

zoranbosnjak · 2017-07-28T14:23:41Z

There was a conceptual problem with validation function, since it assumes "valid" for all unknown instances. This in turn generates valid status for some otherwise false imputs (false positives). This implementation instead assumes "not implemented" unless "errors" method is implemented on all subitems.

This change introduces small incompatibility with previous version.

is_valid function is removed
obj.is_valid property is introduced instead
obj.errors() method is introduced to get error details

Test cases and README file is changed to reflect the change.

I have only small set of geojson data available. I suggest to run validation tests on bigger dataset if you have it available.

Use obj.is_valid to get object validation status (Bool). Use obj.errors() to get an error or collection of errors during validation The return type depends on the object, but in any case, bool(obj.errors()) must evaluate to: True in case of error(s) False if no errors This implementation either checks for all subitems or raises NotImplementedError.

frewsxcv · 2017-07-30T10:24:28Z

geojson/base.py

+    def checkListErrors(self, checkFunc, lst):
+        """Validation helper function."""
+        # check for errors on each subitem, filter only subitems with errors
+        results = [checkFunc(i) for i in lst]


This could benefit from being a generator:

results = (checkFunc(i) for i in lst)

frewsxcv · 2017-07-30T10:27:39Z

geojson/base.py

+    def is_valid(self):
+        return not self.errors()
+
+    def checkListErrors(self, checkFunc, lst):


Can we call this check_list_errors? We're not very consistent right now, but it'd be great to have everything be snake case

frewsxcv · 2017-07-30T10:28:41Z

CHANGELOG.rst

@@ -1,6 +1,11 @@
 Changes
 =======

+1.4.0 (2017-07-28)


This is a breaking change, so we should bump to 2.0.0 🎊 🎉 2️⃣ .0️⃣

frewsxcv · 2017-07-30T20:56:27Z

geojson/geometry.py



-class Polygon(Geometry):
-    pass
+def checkPolygon(coord):


frewsxcv · 2017-07-30T20:56:44Z

geojson/geometry.py

-class Polygon(Geometry):
-    pass
+def checkPolygon(coord):
+    lengths = all([len(elem) >= 4 for elem in coord])


all([len(elem) >= 4 for elem in coord])

zoranbosnjak · 2017-07-31T06:43:09Z

I have fixed the comments, except I did not understand last remark:

-class Polygon(Geometry):
-    pass
+def checkPolygon(coord):
+    lengths = all([len(elem) >= 4 for elem in coord])

all([len(elem) >= 4 for elem in coord])

What needs to be changed here?

frewsxcv · 2017-08-01T23:58:02Z

What needs to be changed here?

woops, meant to write:

all(len(elem) >= 4 for elem in coord])

Also, do you happen to know if your changes fix #99 ?

zoranbosnjak · 2017-08-04T08:40:45Z

I have converted from list to generator.

Regarding the issue #99, the validation now raises attribute error.
It turns out that the features attribute is a "dict" in this partucilar case, not GeoJSON instance.

Check this:

import geojson

fc = geojson.loads("""{
    "type": "FeatureCollection",
    "features": [{}]
    }
""")

print type(fc.features[0])
print fc.errors()

>>> <type 'dict'>

I am not sure, but it might be better to fix #99 in the init, so that features list is a list of Feature instances. The validation would in this case turn False on the Feature instance and also on toplevel FeatureCollection object.

frewsxcv

everything looks good to me here, thanks for the pr! before merging, you'll need to address some style/flake8 things:

https://travis-ci.org/frewsxcv/python-geojson/jobs/260921306

frewsxcv · 2017-08-05T12:56:39Z

geojson/geometry.py

+    if lengths is False:
+        return 'LinearRing must contain with 4 or more positions'
+
+    isring = all([elem[0] == elem[-1] for elem in coord])


isring = all(elem[0] == elem[-1] for elem in coord)

codecov-io · 2017-08-06T06:32:02Z

Codecov Report

Merging #98 into master will decrease coverage by 2.81%.
The diff coverage is 84.09%.

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
- Coverage   81.23%   78.41%   -2.82%     
==========================================
  Files          12       11       -1     
  Lines         325      315      -10     
  Branches       60       52       -8     
==========================================
- Hits          264      247      -17     
- Misses         51       58       +7     
  Partials       10       10

Impacted Files	Coverage Δ
geojson/__init__.py	`100% <ø> (ø)`	⬆️
geojson/_version.py	`100% <100%> (ø)`	⬆️
geojson/geometry.py	`96.61% <100%> (+1.48%)`	⬆️
geojson/feature.py	`82.35% <40%> (-17.65%)`	⬇️
geojson/base.py	`78.33% <60%> (-3.67%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 112e457...10c9713. Read the comment docs.

frewsxcv · 2017-08-12T23:32:14Z

thanks!

frewsxcv · 2017-08-12T23:36:33Z

just published 2.0.0

This involved some breaking changes, especially during the validation. See jazzband/geojson#98 for details.

With geojson version 2.0.0 the function 'is_valid' was removed. * jazzband/geojson#98 * https://github.com/jazzband/python-geojson/blob/master/CHANGELOG.rst#200-2017-07-28

The geojson library removed the geojson.is_valid function with version 2.0.0. * https://github.com/jazzband/python-geojson/blob/master/CHANGELOG.rst#200-2017-07-28 * jazzband/geojson#98

zoranbosnjak mentioned this pull request Jul 28, 2017

Unable to validate FeatureCollection #82

Open

frewsxcv requested changes Jul 30, 2017

View reviewed changes

review changes

0664ed7

additional review change

d7c411e

frewsxcv requested changes Aug 5, 2017

View reviewed changes

frewsxcv reviewed Aug 5, 2017

View reviewed changes

coding style changes

239b3b1

frewsxcv added 2 commits August 12, 2017 10:40

Use generator comprehension

5e209b0

Get rid of pypy3 for now

10c9713

frewsxcv merged commit 0f782f7 into jazzband:master Aug 12, 2017

pbugnion added a commit to pbugnion/gmaps that referenced this pull request Aug 20, 2017

Migrate to geojson 2.0.0

45f468b

This involved some breaking changes, especially during the validation. See jazzband/geojson#98 for details.

pyup-bot mentioned this pull request Nov 26, 2017

Pin geojson to latest version 2.3.0 openearth/flowmap#15

Merged

pyup-bot mentioned this pull request Mar 6, 2018

Pin geojson to latest version 2.3.0 isogeo/isogeo-api-py-minsdk#14

Closed

p-leger mentioned this pull request Jul 17, 2019

Pin geojson version to 1.3.5: CxAalto/gtfspy#20

Closed

p-leger mentioned this pull request Jul 17, 2019

Fix test_exports: use obj.is_valid. CxAalto/gtfspy#21

Merged

pyup-bot mentioned this pull request Jun 30, 2020

Pin geojson to latest version 2.5.0 camptocamp/c2cgeoportal#6637

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rewrite of validation mechanism #98

rewrite of validation mechanism #98

zoranbosnjak commented Jul 28, 2017

frewsxcv Jul 30, 2017

frewsxcv Jul 30, 2017

frewsxcv Jul 30, 2017

frewsxcv Jul 30, 2017

frewsxcv Jul 30, 2017

zoranbosnjak commented Jul 31, 2017

frewsxcv commented Aug 1, 2017

zoranbosnjak commented Aug 4, 2017

frewsxcv left a comment

frewsxcv Aug 5, 2017

codecov-io commented Aug 6, 2017 •

edited

frewsxcv commented Aug 12, 2017

frewsxcv commented Aug 12, 2017

rewrite of validation mechanism #98

rewrite of validation mechanism #98

Conversation

zoranbosnjak commented Jul 28, 2017

frewsxcv Jul 30, 2017

Choose a reason for hiding this comment

frewsxcv Jul 30, 2017

Choose a reason for hiding this comment

frewsxcv Jul 30, 2017

Choose a reason for hiding this comment

frewsxcv Jul 30, 2017

Choose a reason for hiding this comment

frewsxcv Jul 30, 2017

Choose a reason for hiding this comment

zoranbosnjak commented Jul 31, 2017

frewsxcv commented Aug 1, 2017

zoranbosnjak commented Aug 4, 2017

frewsxcv left a comment

Choose a reason for hiding this comment

frewsxcv Aug 5, 2017

Choose a reason for hiding this comment

codecov-io commented Aug 6, 2017 • edited

Codecov Report

frewsxcv commented Aug 12, 2017

frewsxcv commented Aug 12, 2017

codecov-io commented Aug 6, 2017 •

edited