Featureset should be a list instead of using a next() iterator, feature.properties should be a dictionary #280

Closed
artemp opened this Issue Oct 11, 2011 · 5 comments

Comments

Projects
None yet
1 participant
Owner

artemp commented Oct 11, 2011

This would be way cool compared to the current next() implementation - ie have this class be much more pythonic.

Owner

artemp commented Oct 11, 2011

[Beau Gunderson] Attached a patch to mapnik_featureset.cpp; it works but I doubt it's a 'good' solution but I figured I would share so someone else could see in case it helps. A better solution would be to define {{{len}}} and {{{getitem}}} as described here:

http://wiki.python.org/moin/boost.python/StlContainers

That way the function could return a list instead of returning an object that has a list as a property. I think this requires getting the features into something like a std::vector first. Maybe to_python_converter comes into play here? I am still looking through the boost docs for an example of something similar.

feature.properties is a good candidate for the approach in that URL as well since it already uses a std::map.

Owner

artemp commented Oct 11, 2011

[Beau Gunderson] To test the attached patch you can use this:

{{{
#!python
import mapnik

s = mapnik.Shapefile(file='demo/data/boundaries')
f = s.features_at_point(s.envelope().center())
}}}

Now you'll have access to {{{f.features}}}.

Owner

artemp commented Oct 11, 2011

[springmeyer] Beau,

Yes, I agree that the vector indexing suite might make sense given that it automatically provides all the list-ish methods for a std::vector, and that it would be handy if the featureset its self were the list. But it does not work this way in the C++ core, so I think that this method of offering a boost::python::list as a fs.features property makes a lot of sense and surely solves the interface problem. This way we can easily add more adjacent methods in the future.

Owner

artemp commented Oct 11, 2011

[springmeyer] revised patch added in r1121, thanks beau!

Owner

artemp commented Oct 11, 2011

[springmeyer] changing milestone... going to be part of 0.6.1 release

artemp closed this Oct 11, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment