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. best, Pierre
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
