Re: [matplotlib-devel] Pull request

[Todd <tod...@gm...>]

On Mon, Oct 21, 2013 at 3:13 PM, Pierre Haessig <pie...@cr...>wrote:

> Hi,
>
> Le 20/10/2013 09:45, Todd a écrit :
> > I submitted a pull request #2522 [1].  It includes support for more
> > basic spectrum plots like magnitude and phase spectrums.  These are
> > extremely commonly used in signal processing, acoustics, and many
> > other fields, but are also very important for educational usage since
> > pretty much anyone going through one of several engineering degrees
> > like electrical engineering has to learn these types of plots.  I have
> > heard a number of colleagues complaining about the lack of these plots
> > in matlab.
>
> Having specific signal processing functions is indeed important. I hust
> have a question about "phase" vs. "angle" spectrum. From browsing
> quickly through you doc, it seems that the difference is just about
> *unwrapping the phase*. If that's indeed the case, I've two
> questions/remarks:
>
> 1) is the terminology "phase" vs. "angle" spectrum standardized ? I must
> say I've never heard of one meaning "wrapped" and the other "unwrapped".
> I didn't find similar terms in Matlab docs, but I didn't search that
> thoroughly.
>

The "angle" function in numpy returns the wrapped angle, while the "unwrap"
function documentation talks about phase, so it is consistent with the
usage in numpy.   Further, in signal processing, phases can have any value,
while "angle" often refers to the angle between two lines, which must be
wrapped.

There may be some ambiguity, but I made sure to explain it in the
documentation and provide links between the two functions so people know
what they should do if they want to use the other approach.


> 2) Should there be two separate functions for these two, or just one
> function, with a switch argument `unwrap` ? (I guess it would be True by
> default)
>

I originally was going to do that, but decided against it.  The problem is
with specgram.  Here, I thought it would be needlessly complicated to add
an "unwrap" parameter that is only useful for one "mode".  To make it
obvious to users, I wanted to keep specgram as similar as possible to the
other plot types, and that involved keeping the parameter.

Further, this approach is simpler to code and easier to maintain. Having to
deal with the "unwrap" parameter would have been more difficult to
program.  Dealing with both an "unwrap" parameter in some cases and a
separate "mode" in others would have been even more complicated.

Further, _spectral_helper and specgram already have a huge number of
arguments.  This way I was able to get away with just adding one new
argument rather than two.