search

JOSM / geojson

Allows reading GeoJSON using different projections – NOW PART OF JOSM CORE
Apache License 2.0
13 stars 10 forks source link

Convert plugin to json-p API #16

Closed don-vip closed 6 years ago

don-vip commented 6 years ago

This plugin currently uses Jackson to convert GeoJSON. While it's surely easier to write it this way, it is a no-go for inclusion in JOSM core (#10), as JOSM already embeds json-p as JSON library. Once the plugin will migrate to this library it will be considered for inclusion.

jreijn commented 6 years ago

If it's still up for grab I would like to do this :-)

don-vip commented 6 years ago

Thank you @jreijn :+1: You're the first one to show up, please go ahead :) If you're new to JOSM development, here is the developer guide. Feel free to ask any question :)

floscher commented 6 years ago

@jreijn Great, yes go ahead. For your information: The GeoJSON plugin can be built using either Ant or Gradle, you can choose which one to use. The developer guide mainly covers the Ant way, for documentation about building with Gradle see https://gitlab.com/floscher/gradle-josm-plugin#readme or ask me.

Happy Hacking!

jreijn commented 6 years ago

@floscher I'm not sure if you know about it, but I was wondering if there are any GeoJSON based POJOs available somewhere in the project? Otherwise I will create some myself.

floscher commented 6 years ago

@jreijn For GeoJSON primitives the classes from Jackson were used: https://github.com/JOSM/geojson/blob/14eab01cf9a24f2e2bd8c18ff7f40fe613695778/src/main/java/org/openstreetmap/josm/plugins/geojson/DataSetBuilder.java#L10-L20 So yes, you can create your own for use with the json-p API. If it's easily possible, maybe you could even directly create (J)OSM primitives without first instantiating objects representing the GeoJSON datastructure. But I'd leave that to you, how you want to implement it.

jreijn commented 6 years ago

@floscher yeah I noticed those, but they are dependent on an external library that's build for Jackson. I'll see what I can do.

don-vip commented 6 years ago

For reference you can take a look to OsmJsonReader core class (which reads an Overpass API JSON OSM file, not a GeoJson). It might be very similar.

jreijn commented 6 years ago

@don-vip wow great. Looks interesting!

jreijn commented 6 years ago

Sorry for the delay. I was sick for a week. Let me know if it requires further improvements. I've tried the plugin with several geo json files and it seems to work nicely. I aso did some minor check style improvements.

don-vip commented 6 years ago

Don't be sorry, thanks a lot for your work! I'm going to take a look :)

don-vip commented 6 years ago

An optimization is missing: the jsonp classes should not be embedded into plugin jar file, as they are provided by JOSM core.

jreijn commented 6 years ago

Ah yes. You are right. I guess they can use scope provided. Do you want me to create a new PR for that?

don-vip commented 6 years ago

Yes, please :)