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

Reply via email to