Hello,
there are 2 points where I disagree with you:
- breaking the code that works with 2D networks when run on a 3D network is
not necessarily a good thing. There is any number of reasonable things to
be done with 2D geometry in a 3D network.
- controlling return types by parameter is less readable than having
dedicated function names. consider reading the following statements in
source:
pos = node.getPosition(true)
vs
pos = node.getPosition3D()
Note only is the 'true' surprising (why would a position function have an
extra parameter?), it also doesn't reveal what it is doing.
By your suggestion, all "clean" scripts would use getPosition(true) from
now on.
I will stand by the implementation plan 3) from my previous mail of having
getPosition() behave as before and getPosition3D() always returning a
3-tuple.
regards,
Jakob
2016-11-16 11:01 GMT+01:00 sumo.mahei <sumo.ma...@googlemail.com>:
> Hi Jacob,
>
> I was thinking about something like:
>
> def getPosition('node_id', withZ=False):
> if len(self._coord) == 2 and withZ: # fake the non existing
> z-coord
> return (self._coords[0], self.coord[1], 0)
> else: #
> return 3d if node is 3d (also for withZ)
> return self._coord # return 2d fi
> node is 2d and not withZ
>
> If you want your new code/scripts to be clean, then use withZ=True from
> now on.
> This always returns x,y,z - save on any net and less code in your script.
>
> It doesn't break your old code on pure 2d nets,
> because your scripts have been eating 2d before and
> are feet now again with 2d only since the net is 2d only.
>
> It will break your old scripts on 3d-networks,
> which is __good__ because quite likely you've done
> something wrong in the past.
>
> If that was on purpose,
> well then use the 2D-Wrapper with search and replace as you suggested.
>
>
> Am 16.11.2016 um 08:45 schrieb Jakob Erdmann:
> > Hello,
> >
> > 1) the advantage of having 3D and 2D variants of geometry are threefold:
> > - it follows the TraCI API which provides 2 distinct methods for
> > returning 2D and 3D geometry
> > (http://sumo.dlr.de/wiki/TraCI/Vehicle_Value_Retrieval). This is only
> > implemented for traci.person.getPosition / getPosition3D in the python
> > client at the moment in case you are wondering why you never saw it.
> -> I experienced this as a drawback of TraCI, I do more look at SUMO and
> netconvert than on TraCI as reference
>
> - it allows trivial search/replace conversion for existing code
> -> agree, see below
>
> > - it follows the coding style in src/utils/geom with distinct methods
> > for handling 2D and 3D geometry
> -> not my playground so far :-)
> -> we may additionally include wrappers with endings 2D and 3D if that
> closes the gap
>
> >
> > 2) the disadvantage of determining function return types at runtime,
> > is that it makes code that handles arbitrary networks much harder to
> > write (you suddenly need to check types when calling geometry
> > functions as your example shows)
> -> agree 100% and agian 100% that is why suggested the use withZ=True as
> described yesterday and again above for arbitrary net-scripting
> b.t.w. checking always the dim is truly very annoying and that is the
> reason why I did write some sumolibExtensions some time ago.
>
> > 3) you raise an important point regarding backwards compatibility.
> > I'm not sure how many people use tuple unpacking in their code but I
> > agree that it's bad to break working code.
> -> ??? you can never tell :-) I know you did use unpacking in your
> scripts :-)
>
> > An alternative would be to have the old client methods continue to
> > return 2D geometry and only add a new method with the 3D suffix
> > (exactly as in traci.person but somewhat different than in
> > src/utils/geom).
> >
> -> this might let your (future) users to stumble across the missing
> z-coord where they expect them. They might think this is a bug unless
> they dive into the docs
> to find out. Not the nicest coding experience.
>
> > best regards,
> > Jakob
> >
> >
>
> So which way to go?
> I suggest:
>
> * return geometry tuples according to the largest dim of the net
> (either always 2d on a pure 2d net or always 3d on a net with at
> least one node's z!=0)
> -> requires a tiny post process after loading the xml data.
>
> * geometry return can switch on the z-layer optionally for 2d nets for
> ease of coding
>
> * have getSomething2D() and getSomething3D() - Wrappers for all geometry
> returning functions,
> brute forcing 2d and 3d tuples even if information is lost.
>
> Would that be fine for you?
>
>
> Best Marek Heinrich
> simoserv GmbH
>
>
>
> >
> >
> >
> >
> > 2016-11-15 19:11 GMT+01:00 sumo.mahei <sumo.ma...@googlemail.com
> > <mailto:sumo.ma...@googlemail.com>>:
> >
> > Dear Jacob,
> >
> > maybe it's worth putting the discussion about Ticket 2072
> > from private mailing to the sumo-devel list.
> >
> > We are currently facing three alternatives for
> > introducing 3d (x,y,z)-coordinates to the sumolib:
> >
> > Alternative 0 - sumolib resemble the sumo.net.xml/netconvert
> behaviour
> > ============================================================
> ==========
> >
> > netconvert has the ability to eat z-coordinates during the
> > netbuilding process for z=0.
> >
> > This is how SUMO/netconvert is, even though I find this annoying
> > whenever
> > I come along this (often enough).
> > Here is an example, which is also part of yesterdays
> > sumolib3d-unittest:
> >
> > The node-file includes 3D-coordinates:
> >
> > <node id="first" x="100.0" y="0.0" z="0.0" />
> > <node id="second" x="200.0" y="0.0" z="10.0" />
> >
> > Once netconvert has finished it's work, the sumo.net.xml contains
> > for instance the following items:
> >
> > <junction id="first" type="priority" x="100.00" y="0.00"
> > incLanes="the_other_way_0" intLanes=":first_0_0" shape="100.00,-0.05
> > 100.00,-2.05 100.00,2.05 100.00,0.05">
> >
> > <junction id="second" type="priority" x="200.00" y="0.00" z="10.00"
> > incLanes="this_way_0" intLanes=":second_0_0"
> > shape="200.00,0.05,10.00 200.0\
> > 0,2.05,10.00 200.00,-2.05,10.00 200.00,-0.05,10.00">
> >
> > <lane id="this_way_0" index="0" speed="13.89" length="100.50"
> > width="2.00" shape="100.00,-1.05 200.00,-1.05,10.00"/>
> >
> > As you can see here, the 'first' point is pure 3d with z=0 and
> > results in a pure 2d 'first' junction.
> >
> > No so with 'second' as z is different from zero.
> >
> > The lane 'this_way_0' is 2.5d since it starts its shape with a
> 2-tuple
> > and continues in full 3d.
> >
> > I can fully understand that this helpful for backward compatibility ,
> > but it also introduces extra work, e.g. when processing
> > sumo.net.xml files
> > in third application.
> >
> > Fixing this issue in netconvert is not in my scope and probably
> > also not
> > in the scope of the ticket :-) and maybe even a different
> discussion.
> >
> > My conclusion about alternative 0:
> > I believe it's normal to expect the sumolib to behave like
> > sumo/netconvert
> > itself, even though I don't like this behaviour.
> >
> >
> > Alternative 1 (dlr-team):
> > =========================
> >
> > This is how I do understand your amendment to Ticket 2072:
> >
> > any geometry functions should always return 3-tuple, regardless of
> the
> > dimensions of the net - also for 2d-nets.
> >
> > Pro: This is great for all nets that contain 3d-Nodes.
> > Pro: This makes all users and developers of new script aware
> > that they might consider 3d-Networks as well
> >
> > Con: This will break all existing scripts, once the user do
> > update to a new version of SUMO, since up to now
> > never 3-tuple were returned even on 3d-networks.
> > This will result in thousands of:
> > ValueError: too many values to unpack
> >
> > This will also break all strips running on 2d-networks.
> > Even though it's perfectly fine to return 2-tuples on a
> > 2d-Networks (Node only having x,y coords only).
> >
> > My personal feeling is that most Networks are in 2d.
> >
> > My conclusion about alternative 1:
> > This is a lot of avoidable trouble and extra work for your
> > users and coders, so please don't stick to it.
> >
> > Alternative 2 (mixture):
> > =========================
> >
> > Why not loading all nodes, and keep a record on the net-object
> > about its dimension.
> >
> > As long as there are no 3d-nodes read by startElement()
> > the net remains as 2d net.
> > -> net has either only 2d nodes or all 3d-nodes are in xy-pane.
> >
> > Once the first node with three coords is read, the net property
> > 'dimension' is changed from 2 to 3.
> >
> > After finishing reading all nodes from xml, the
> > nodes will be processed and missing z-coords will be filled with
> > z=0.
> >
> > If the user accesses now geometry values she/he will be returned
> > either 2- or 3-tuple, depending on the net he is loading.
> >
> > Rather then having a 2D-Wrapper (see below) i would suggest
> > a option 'with_z=True' returning optional the missing z=0 coord on
> > a 2d net.
> >
> > btw. in python3.5 this recipe would work:
> > x,y,z = net.getCorrds() if net.getDim() == 3 else *net.getCoords(), 0
> >
> >
> > A word about backward compatibility with 2D wrapper
> > ================================================
> >
> > I don't see the point in having a extra function getShape2D()
> > or getPositon2d() and so on.
> >
> > As already in the ticket's amendment described, it's as easy
> > for a sumolib user to adapt the code
> > from:
> > x,y = getSomething()
> >
> > to:
> > x,y,z = getSomething()
> >
> > it is more straight forward - even if z as
> > variable is never used again
> >
> > I would argument that adding a wrapper whatEverFunction2D()
> > to the whatEverFunction() might be of little effort, but still
> > in my opinion it will introduce dead code - But this is at your
> > choice.
> >
> >
> > Geometries in TraCI
> > ===================
> > B.t.w. Traci is more strict, it returns 2d coords even for 3d-nets.
> >
> >
> > So which way should we travel?
> >
> > Best Marek Heinrich
> > simoserv GmbH
> >
> >
> > ------------------------------------------------------------
> ------------------
> > _______________________________________________
> > sumo-devel mailing list
> > sumo-devel@lists.sourceforge.net
> > <mailto:sumo-devel@lists.sourceforge.net>
> > https://lists.sourceforge.net/lists/listinfo/sumo-devel
> > <https://lists.sourceforge.net/lists/listinfo/sumo-devel>
> >
> >
>
>
> ------------------------------------------------------------
> ------------------
> _______________________________________________
> sumo-devel mailing list
> sumo-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/sumo-devel
>
------------------------------------------------------------------------------
_______________________________________________
sumo-devel mailing list
sumo-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sumo-devel
