Ticket Navigation

Ticket #132 (defect)

Opened 1 year ago

Last modified 10 months ago

GeoJSON ChangeSet 935 breaks my code

Status: closed (fixed)

Reported by:	elem	Assigned to:	mattrussell
Priority:	critical	Milestone:	geojson-1.0
Component:	GeoJSON	Version:	1.0
Keywords:	encoding	Cc:	mattrussell

Since ChangeSet 395 my code produces this traceback:

Module mapfishsample.controllers.countries:43 in show
<<          objects = search.query(model.Session, model.Country,
model.countries_table, expr)
           if len(objects) != 0:
               return geojson.dumps(FeatureCollection([f.toFeature()
for f in objects]))>>  return
geojson.dumps(FeatureCollection([f.toFeature() for f in objects]))
Module geojson.encoding:134 in dumps
<<
   def dumps(obj, cls=PyGFPEncoder, **kwargs):
       return simplejson.dumps(to_mapping(obj), cls=cls, **kwargs)>>
return simplejson.dumps(to_mapping(obj), cls=cls, **kwargs)
Module simplejson:215 in dumps
Module simplejson.encoder:352 in encode
Module simplejson.encoder:305 in _iterencode
Module simplejson.encoder:297 in _iterencode
Module simplejson.encoder:263 in _iterencode_dict
Module simplejson.encoder:294 in _iterencode
Module simplejson.encoder:192 in _iterencode_list
Module simplejson.encoder:305 in _iterencode
Module simplejson.encoder:311 in _iterencode_default
Module geojson.encoding:123 in default
<<
       def default(self, obj):
           return to_geojson_dict(obj)>>  return to_geojson_dict(obj)
Module geojson.encoding:59 in to_geojson_dict
<<      The resulting value will have GeoJSON's defaults applied.
       """
       d = dict(mapping)
       type_ = d.pop("type", None)
       if type_:>>  d = dict(mapping)
exceptions.TypeError: iteration over non-sequence

I attach my pfpfeature.py file which contains the definitions of Feature and FeatureCollection?.

Attachments

pfpfeature.py (1.7 kB) - added by elem on 10/31/07 08:20:07.
patch-geojson-132-A0.diff (413 bytes) - added by elem on 10/31/07 08:50:01.: encoding.py patch
patch-geojson-132-B0.diff (1.0 kB) - added by elem on 10/31/07 08:59:07.: patch to geometry.py
c2c_features.txt (1.3 kB) - added by seang on 10/31/07 09:57:08.
geojson-mattrussell-workingcopy-to-r935.diff (20.1 kB) - added by mattrussell on 10/31/07 20:04:43.: And the diff
geojson-fixes132-and-tests-more-allows-object-or-dict.tgz (41.6 kB) - added by mattrussell on 10/31/07 20:21:04.: Upadted to include c2c_features.txt
geojson-mattrussell-workingcopy-to-r935.2.diff (20.1 kB) - added by mattrussell on 10/31/07 20:21:58.: Upadted to include c2c_features.txt
geojson-mattrussell-workingcopy-to-r935.3.diff (20.1 kB) - added by mattrussell on 10/31/07 20:24:21.: diff again after svn up
geojson-mattrussell-workingcopy-to-r935.4.diff (21.7 kB) - added by mattrussell on 10/31/07 20:26:29.: arg. I've changed one line I belive to be incorrect in the test
geojson-mattrussell-workingcopy-to-r939.diff (23.8 kB) - added by mattrussell on 11/01/07 12:08:56.: Removes check* methods from geojson.geometry
geojson-updates-from-r939.tgz (40.0 kB) - added by mattrussell on 11/01/07 12:18:12.: Removes check* methods from geojson.geometry and removes spurious file entries from the previous tarball
GeoJSONr944-to-mgrWorkingCopy.diff (23.7 kB) - added by mattrussell on 11/12/07 05:44:54.: Updated patch

Change History

10/31/07 08:20:07: Modified by elem

attachment pfpfeature.py added.

10/31/07 08:50:01: Modified by elem

attachment patch-geojson-132-A0.diff added.

encoding.py patch

10/31/07 08:51:55: Modified by elem

I attached a patch to encoding.py. That patch partially solves my problem but I have some other issues with the new code. These other issues might be related to the fact that the new Geometry class doesn't handle polygons with holes I think.

10/31/07 08:59:07: Modified by elem

attachment patch-geojson-132-B0.diff added.

patch to geometry.py

10/31/07 09:01:50: Modified by elem

I attached a new patch. The combination of the two patches makes my issues go away. However, I'll let you guys check my patches as I'm not sure at all about what I did :-)

10/31/07 09:54:35: Modified by seang

cc set to mattrussell.
keywords set to encoding.
status changed from new to assigned.

Eric, I'm inclined to accept the patches. Matt, could you try them out?

In r939 I committed a test of c2c features and collections and I'm attaching a new version of the tests that work with Eric's patches. Eric, will you review the attached test and check that it matches your classes?

10/31/07 09:57:08: Modified by seang

attachment c2c_features.txt added.

10/31/07 16:23:15: Modified by seang

On second thought: do we want to allow objects that provide the geo interface into the geo interface? Should

{'id': '1', 'geometry': shapely.geometry.Point(0,0), ...}

be just as valid as

{'id': '1', 'geometry': {'type': 'Point', 'coordinates': (0, 0)}, ...}

10/31/07 20:00:24: Modified by mattrussell

status changed from assigned to closed.
resolution set to fixed.

I think yes Sean :)

I have attached working patch that implements this, fixes 132, and adds tests for featurecollections.

10/31/07 20:02:42: Modified by mattrussell

status changed from closed to reopened.
resolution deleted.

Sorry, not my call to say 'fixed' yet really is it! Sorry Sean...

10/31/07 20:04:43: Modified by mattrussell

attachment geojson-mattrussell-workingcopy-to-r935.diff added.

And the diff

10/31/07 20:21:04: Modified by mattrussell

attachment geojson-fixes132-and-tests-more-allows-object-or-dict.tgz added.

Upadted to include c2c_features.txt

10/31/07 20:21:58: Modified by mattrussell

attachment geojson-mattrussell-workingcopy-to-r935.2.diff added.

Upadted to include c2c_features.txt

10/31/07 20:24:21: Modified by mattrussell

attachment geojson-mattrussell-workingcopy-to-r935.3.diff added.

diff again after svn up

10/31/07 20:26:29: Modified by mattrussell

attachment geojson-mattrussell-workingcopy-to-r935.4.diff added.

arg. I've changed one line I belive to be incorrect in the test

11/01/07 06:49:28: Modified by elem

Hi Matt

I saw your email saying that you want to remove the geometry validation. Anyway, I wanted to let you know that the polygon check method doesn't support polygons with holes:

class Polygon(Geometry): pass
    @classmethod
    def check(cls, coordinates):
        # check it's a ring
        (first, last) = (coordinates[0], coordinates[-1])
        if round(last - first, self._decimal_precision) == 0:
            raise ValueError("Invalid Polygon, must form a ring.")
        MultiPoint.check(coordinates)

11/01/07 12:07:54: Modified by mattrussell

Hi Eric,

Thanks, I saw your code which fixes the check method, and you are correct the existing check method for polygon doesn't support holes. As you mention, I don't think this checking belongs in geojson.

I'll attach a tar and diff which removes this, which is against r939.

11/01/07 12:08:56: Modified by mattrussell

attachment geojson-mattrussell-workingcopy-to-r939.diff added.

Removes check* methods from geojson.geometry

11/01/07 12:18:12: Modified by mattrussell

attachment geojson-updates-from-r939.tgz added.

Removes check* methods from geojson.geometry and removes spurious file entries from the previous tarball

11/02/07 13:11:54: Modified by seang

Matt, can you make an 'svn diff' from your work? There are too many files in that tarball, and I'd like to be able to focus on the significant changes.

11/06/07 10:44:19: Modified by seang

milestone set to GeoJSON 1.0.

11/12/07 05:44:54: Modified by mattrussell

attachment GeoJSONr944-to-mgrWorkingCopy.diff added.

Updated patch

11/15/07 00:13:49: Modified by seang

OK, I did the best merge I could. Can you try rev 946?

Matt, I had to remove the absolute/relative imports. I'm doing a lot of development on 2.4.4.

03/06/08 04:58:39: Modified by mattrussell

This has now been fixed in the trunk and works with Python2.4 and Python2.5. Sean, mind if I re-assign to me and close?

03/11/08 08:30:22: Modified by seang

owner changed from seang to mattrussell.
status changed from reopened to new.

03/11/08 09:14:33: Modified by mattrussell

status changed from new to closed.
version set to 1.0.
resolution set to fixed.