search

marook / osm-read

an openstreetmap XML and PBF data parser for node.js and the browser
GNU Lesser General Public License v3.0
107 stars 25 forks source link

Relation members output format #20

Closed nrenner closed 10 years ago

nrenner commented 10 years ago

The relation members format introduced in #17 uses a separate array for each member type: relationsMembers = { nodes: [], ways: [] };

The problem is, that the members of a relation are an ordered list, regardless of type, that cannot be reconstructed from this output. That is, this output format loses information.

Therefore I suggest to use a single members array where each element contains a type property with one of node, way or relation as value. This is what other libs use as well: openstreetmap-json-schema (example), Overpass API JSON, osmtogeojson.

What do you think?

marook commented 10 years ago

I agree with you. I think it's a bad idea to lose the relation member orderings over different types. I think we should break backwards compatibility in order to adjust the relations interface.

I've committed an interface proposal in the 'relations' branch. The commit which changes the expected interface just in the unit tests is https://github.com/marook/osm-read/commit/5db2f975384943e6d3b42230ca774364b3965f7a

Maybe it's possible to keep the former nodes and ways properties as deprecated members of the relationsMembers element. This will create a little memory overhead. But I think I'm willing to spend that for the sake of backward compatibility.

What do you think?

nrenner commented 10 years ago

I would like to avoid any memory overhead and rather not support both variants at the same time. But I only care about PBF and as relation support for PBF has not been released yet, I would prefer to change that implementation to only support a single members array.

Not sure how many users there are for the new XML relations and if breaking backwards compatibility would be ok. @BinaryBrain ?

BinaryBrain commented 10 years ago

My implementation should not cause trouble for people who don't handle relations. If there is no callback, nothing will happen. It's like if you don't handle way because you only use nodes. By the way, I don't see why you would need way and not relations.

nrenner commented 10 years ago

@BinaryBrain I implemented relation support for PBF following your implementation. My question now is, if it would be Ok to change the returned relation members object (and thus break the API) to contain only a single array with a type property instead of separate arrays for each type, because with the latter we loose the overall member ordering (see my first comment above).

BinaryBrain commented 10 years ago

Well you seem do know OSM better than me and you also seem to be right. Feel free to modify the format as you want, there's no problem for me.

That said, maybe you should be careful of the retro-compatibility. If that's impossible, maybe you should create a new osmread.parse(...) method

marook commented 10 years ago

I've changed the API of the relation callback.

@BinaryBrain In order to use the next version (v0.5 (At the time of writings it's not yet released)) of osm-read you will need to adjust some parts of your relations code which calls osm-read. I've added some description about the required changes in the file ChangeLog. Hope that helps.