[osmosis-dev] Fwd: osmchange file produced by osmconvert gives osmosis error

[Martijn van Exel]

Relaying this response from Markus.

I was confused by the osmosis error. Great that osmconvert offers the
--fake-latlon but the lat and lon being required in this case is super
confusing (as is the osmosis error message), probably not only to me.

Martijn

---------- Forwarded message ----------
From:  <markus.weber at gmx.com>
Date: Tue, Aug 7, 2012 at 11:33 AM
Subject: Re: [osmosis-dev] osmchange file produced by osmconvert gives
osmosis error
To: Igor Podolskiy <igor.podolskiy at vwi-stuttgart.de>
Cc: m at rtijn.org


Hello Igor,

thanks for this very elaborated statement!

As far as I am concerned - resp. as far as osmconvert is concerned -
you do not need to worry because most users will set the --fake-lonlat
option when exporting .osc files if they are going to be processed by
Osmosis afterwards:
https://wiki.openstreetmap.org/wiki/Osmconvert#Retrieving_the_Differences_between_two_OSM_Files

Nevertheless, please check if you could improve the error message.
Martijn told me, Osmosis threw a hardly expressive
"java.lang.NullPointerException" when being fed with .osc files with
no lon/lat.

Greetings from Nürnberg
Markus


P.S.: I'm not a subscriber of osmosis-dev. If you think it's useful,
please relay this email.


-------- Original-Nachricht --------
> Datum: Tue, 7 Aug 2012 14:39:49 +0200
> Von: Igor Podolskiy <igor.podolskiy at vwi-stuttgart.de>
> An: osmosis-dev at openstreetmap.org, Martijn van Exel <m at rtijn.org>
> CC: markus.weber at gmx.com
> Betreff: Re: [osmosis-dev] osmchange file produced by osmconvert gives osmosis error

> Hi,
>
> On 04.08.2012 07:01, Martijn van Exel wrote:
> > I produced this small osmchange file using a recently compiled
> > osmconvert: https://gist.github.com/3254628
> > the osmosis --rxc task chokes on it though:
> https://gist.github.com/3254632
> >
> > Ideas?
> yes, the cause of this is pretty simple. Osmosis requires lat and lon
> attributes on nodes, and these attributes are not set in the change file
> on the deleted nodes.
>
> While the cause is pretty simple, the fix for this (_if_ we want to fix
> this) isn't going to be simple. The abscence of the lat/lon attributes
> on the deleted nodes is conceptually OK since it is going to be ignored
> anyway. However, the <node> parser (NodeElementProcessor to be precise)
> does not know about the context of the <node> element - and the
> osmChange <delete> is the only case where lat/lon is actually optional.
>
> Also, the Osmosis data model (the Node class) has an implicit assumption
> that the nodes always have lat/lon - the fields are typed as double and
> can't thus be set to null, and there is no such thing like
> Node.isPositionDefined().
>
> There also was a recent related API change which makes lat/lon optional
> for already deleted (visible="false") nodes, so now seems a good point
> to talk about that too.
>
> So what can be done about this:
>
> 1. Make the lat/lon attributes optional and change the data model
> accordingly.
> 2. Make the lat/lon attributes optional everywhere and don't change the
> data model.
> 3. Teach the NodeElementProcessor about context it is called from and
> make it generate some bogus and/or sentinel values for the Node classes
> in that case (0, Double.NaN, Double.MAX_VALUE, whatever).
> 4. Ignore the whole issue and just say that lat/lon is required.
>
> My personal and also very humble opinion about those:
>
> 1.: For this, we would need to change all the downstream tasks to check
> if the position is actually there. Given that the correct absence of
> lat/lon is a quite rare case, this seems rather costly.
> 2. and 3.: These are actually almost the same thing. If only the parser
> thinks lat/lon are optional and the downstream tasks think it's always
> there, the parser needs to put some value in the node for
> Node.getLatitude() to return.
>
> The only difference between 2 and 3 is the amount of validation done in
> the parser (and therefore the assumptions the downstream tasks can have
> about the data). I think it's better validate in the parser and fail
> fast. Yes, bringing context to NodeElementProcessor is a bit ugly - but
> hey, it's deep down in the implementation classes and nobody sees it. If
> the input is actually broken (normal OSM XML with lon/lat not set), it's
> better to fail in the parser than pretend that it's OK leading to
> undefined behavior downstream.
>
> 4.: Well, we _could_ ignore osmconvert producing those files (no offense
> intended, Markus) and argue that it's a bug in osmconvert and it should
> be fixed on that side and so on. But I don't think it is wise to ignore
> the API... So this is not an option, despite having a very attractive
> implementation cost of zero :)
>
> To sum it up, I tend to prefer 3.
>
> Other thoughts on this point, anyone? :)
>
> Greetings from Stuttgart
> Igor


-- 
martijn van exel
http://oegeo.wordpress.com