Re: [matplotlib-devel] Pull request

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

On Fri, Oct 25, 2013 at 3:06 PM, Pierre Haessig <pie...@cr...>wrote:

>  Hi,
>
> Le 21/10/2013 15:58, Todd a écrit :
>
>   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.
>
>
> You've convinced me. I didn't have the big picture of your PR when writing
> my previous messages. I like the approach you took for specgram, which put
> "magnitude", "phase", "angle" on the same level. This indeed reduce the
> number of keywords.
>
> Coming back to the readability, what do you think of replacing "phase",
> "angle" by "unwrapped phase", "phase". Beyond readability, it also
> emphasizes for the reader the idea of "postprocessing" to get the unwrapped
> phase, i.e. a something that can have it's issue.
>

 I considering that.  The problem is that people have to type all that.
"magnitude_spectrum" is long enough as it is, but
"unwrapped_phase_spectrum" is just a huge function name (24 characters).  I
wanted something more concise.  I think the documentation makes it clear
enough.  I don't think we lose that much, the users only have to read the
docstring once, and they will avoid a lot of annoyance typing out such a
huge function name over and over.