octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #8827] New function for mapping package


From: Philip Nienhuis
Subject: [Octave-patch-tracker] [patch #8827] New function for mapping package
Date: Fri, 11 Dec 2015 12:18:38 +0000
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 SeaMonkey/2.38

Follow-up Comment #2, patch #8827 (project octave):

(Also see discussion here:
http://octave.1599824.n4.nabble.com/wrapToPi-function-tt4673887.html)

@Oscar
Please supply some tests for these functions.
Also use "endfunction" rather than "end" and indent the actual code w. 2
spaces. Also consider what I wrote in Comments (below).

Once done, I'll do some additional but minor style edits before adding them to
the mapping package. I plan to make a new mapping package release quite soon
(next week), hopefully you'll be in time to have these functions included.

Some comments:
==============
I'm not in favor of camel-casing function names. I'd prefer wraptopi or even
wrap2pi rather than wrapToPi) but it seems Matlab introduced this sillyness
here so we'd need to follow for the sake of Matlab compatibility :-)

I tried "wrapToPi ([-pi, -3*pi])" and got [3.1416 3.1416] as answer. AFAIU
that's not in line with what is stated by TMW here:
http://nl.mathworks.com/help/map/ref/wraptopi.html
where they say that "odd, negative multiples of pi map to −pi" => I'd expect
[-3.1416 3.1416]

Similarly,

>> wrapTo360 ([0, 180, 360, -1, 361])
ans =
     0   180   360   259     1
## Shouldn't that be [0 180 360 359 1] ?

>> wrapTo180 ([0, 180, -180, -1, 181])
ans =
     0   180   180    -1  -179
## Expected [0 180 -180 -1 -179]


...which AFAIU indicates that there's some polishing needed to get
Matlab-compatible answers.
(Mathematically I think your functions are mostly correct but Matlab
apparently does something odd regarding closed/open intervals at the lowest
interval extremes. And we'd have to follow that....)


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8827>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]