Well, I seem to have really dove into this. Here are 4 different patches against the latest svn of axes.py (rev 2495). Note that the rest of my install is the 0.87.3 release (I had to copy over quiver.py to get the latest axes.py to work). patch1 has the following changes to bar() and barh(): - fixed ignoring the rcParams['patch.facecolor'] for bar color: the default value for the color arg is now None, and the Patch class is left to handle fetching the rcparams['patch.facecolor'] - set default error bar color to None, so that errorbar() can handle fetching the rcParams['lines.color'] - added an edgecolor keyword arg - left and height can now both be scalars in bar(), same goes for x and y in barh(). Previously, this raised a TypeError upon testing their lengths. Code that preventively checked for this in barh() (but not in bar()) has been removed. - fixed a bug where patches would be cleared when error bars were plotted if rcParams['axes.hold'] was False - it looks like the code for barh() was copied from bar(), with some of the args renamed. There was an error in the color checking code in barh() where len(left) from bar() hadn't been properly renamed to len(x) - found one or two changes that had been made to bar() that hadn't been propagated to barh(), or vice versa - rearranged the order of some code segments so that they follow the order of the arguments - updated the docstrings Hopefully I haven't introduced any new bugs. patch2 has everything in patch1, except it removes some code duplication by calling bar() from within barh(). I thought this would be a good idea, since it's easy to make a change in bar() and forget to do the same in barh(). It turns out that this takes up almost as many lines of code as having two independent functions, but this is only due to inconsistent behaviour: barh() draws bars vertically centered on the y values (ala matlab 6.0), while bar() draws bars aligned according to their left edge (not ala matlab). I prefer the edge aligning behaviour. It's easy to convert from one behaviour to the other, but I had to duplicate all the error checking code before conversion, which bloated it back up. So... patch3 has everything in patch2, but renames the x and y args in barh() to width and bottom respectively. This makes barh() draw bars vertically aligned to their bottom edge, consistent with bar()'s behaviour. Also, this makes hist(orientation='horizontal') do the same, which makes it consistent with hist(orientation='vertical'). Finally, it removes the code bloat mentioned above. However, it'll break any existing code that relies on x or y as named args in barh(), or code that expects barh() bars to be vertically centered on their y values. And lastly... I find it odd that barh() has the width and bottom args (formerly x and y) in that order: barh(width, bottom). The general matlab convention is that the first argument is the positions, and the second arg is the values. So it would make more sense to me to have barh(bottom, width). That way, you could switch back and forth between bar() and barh() and get the expected behaviour without having to switch around the arguments. In fact, that's exactly how barh() in matlab 6 interprets the first two arguments: arg1 is the vertical positions, and arg2 is the lengths of the bars at those positions. Same goes for matlab's bar() function. As it is now in matplotlib, the first and second arguments are interpreted differently for bar() and barh() I don't know if anyone agrees with this change, but patch4 has all of the changes in patch3, plus the order of the width and bottom args are switched in barh(). This of course will break existing code that depends on this order. I had to modify the barh() call in hist(orientation='horizontal') to reflect this. I couldn't find any other barh() call in matplotlib. For consistency, I also switched the order of the yerr and xerr args, but these have default values and are usually passed as keyword args, so this shouldn't break (much) code. The patches are numbered in increasing order of preference. They look rather big (and I'm not sure if my file compare util is bug-free). If there seem to be problems with them, I can provide the full axes.py file that corresponds to each patch. Cheers, Martin
>>>>> "Martin" == Martin Spacek <sc...@ms...> writes: Hey martin, thanks for all these changes. Martin> to inconsistent behaviour: barh() draws bars vertically Martin> centered on the y values (ala matlab 6.0), while bar() Martin> draws bars aligned according to their left edge (not ala Martin> matlab). I prefer the edge aligning behaviour. It's easy Martin> to convert from one behaviour to the other, but I had to Martin> duplicate all the error checking code before conversion, Martin> which bloated it back up. Most people prefer the center aligning behavior, at least those who complained on the list about bar, so when I wrote barh I adopted this. I tried to fix bar in the process, but ended up running into some bugs when I tested John Gill's table demo, and so left it as edge aligned and haven't revisited it since. So my weak preference would be to have the two functions consistent and center aligned, but he who does the work usually gets the biggest vote. Maybe others can chime in. Martin> And lastly... I find it odd that barh() has the width and Martin> bottom args (formerly x and y) in that order: barh(width, Martin> bottom). The general matlab convention is that the first Martin> argument is the positions, and the second arg is the Martin> values. So it would make more sense to me to have Martin> barh(bottom, width). That way, you could switch back and Martin> forth between bar() and barh() and get the expected Martin> behaviour without having to switch around the Martin> arguments. In fact, that's exactly how barh() in matlab 6 Martin> interprets the first two arguments: arg1 is the vertical Martin> positions, and arg2 is the lengths of the bars at those Martin> positions. Same goes for matlab's bar() function. As it is Martin> now in matplotlib, the first and second arguments are Martin> interpreted differently for bar() and barh() I was following the convention that the x arg goes first and the y second, but I'm not wed to this. I don't mind breaking existing code if this order seems more natural, and since we are mostly emulating the matlab conventions in bar and barh, it makes some sense to strive for consistency. Perhaps you could patch the CHANGELOG and API_CHANGES file along with the rest which explains the changes. JDH
Hi John, John Hunter wrote: > Most people prefer the center aligning behavior, at least those who > complained on the list about bar, so when I wrote barh I adopted > this. I tried to fix bar in the process, but ended up running into > some bugs when I tested John Gill's table demo, and so left it as edge > aligned and haven't revisited it since. So my weak preference would > be to have the two functions consistent and center aligned, but he who > does the work usually gets the biggest vote. Maybe others can chime > in. I suppose I'm a bit jaded towards edges because I tend to make histograms and not bar graphs, but we can have it both ways. Here's patch5 (now against the latest axes.py rev 2508). It has everything in patch4, plus a keyword arg 'align' that lets you choose between aligning the bars according to their edges (left for vertical bars, bottom for horizontal bars) or their centers. The default is align='edge' for both bar() and barh(). Perhaps that should be changed to 'center' if most people prefer it that way. Also, the 'horizontal' boolean arg in patch4 has been renamed to 'orientation' and is now a string: either 'vertical' or 'horizontal', consistent with hist(). I also added the align arg to hist(), which just passes it on to bar() or barh(). > I was following the convention that the x arg goes first and the y > second, but I'm not wed to this. In barh(), Matlab does indeed order the args x, y, but interprets them as y, x (ie, position, value), which actually makes sense to me. > Perhaps you could patch the CHANGELOG and > API_CHANGES file along with the rest which explains the changes. Sure. Here they are against their latest rev (2508 for both). Never done logs before, hope they're alright. What do you mean by "the rest"? Cheers, Martin
>>>>> "Martin" == Martin Spacek <sc...@ms...> writes: Martin> I suppose I'm a bit jaded towards edges because I tend to Martin> make histograms and not bar graphs, but we can have it Martin> both ways. I can live with that -- did you test your work with the table_demo? Martin> Sure. Here they are against their latest rev (2508 for Martin> both). Never done logs before, hope they're alright. What Martin> do you mean by "the rest"? "the rest", meaning the work you had already done. I'm having trouble applying your patch because of the way the file names are coded. If somebody knows the magic patch command to make it go through, please commit it. Otherwise, Martin, can you make a patch with svn diff from the mpl root dir (the one that setup.py lives in)? Thanks, JDH
John Hunter wrote: > I can live with that -- did you test your work with the table_demo? I just tried table_demo, looks good, bars are nicely centered (had to set my rcparams axes.hold to True to get all four colours of bars). > I'm having trouble applying your patch because of the way the file > names are coded. If somebody knows the magic patch command to make it > go through, please commit it. Otherwise, Martin, can you make a patch > with svn diff from the mpl root dir (the one that setup.py lives in)? Ah, I was just working off the axes.py file from viewcvs. I've checked out now from /trunk/matplotlib/ (using tortoise svn). Hopefully this new patch file will work. Cheers, Martin
>>>>> "Martin" == Martin Spacek <sc...@ms...> writes: Martin> Ah, I was just working off the axes.py file from Martin> viewcvs. I've checked out now from /trunk/matplotlib/ Martin> (using tortoise svn). Hopefully this new patch file will Martin> work. Making progress - I was able to apply this and check it in, but I only got a patch for axes.py (no CHANGELOG, API_CHANGES, modified examples, etc). Also, examples/barh_demo.py is now broken after application of this partial patch). Could you get a fresh checkout from svn (revision 2515) and see what is missing and then provide an updated patch? Thanks! JDH
Whoops. Forgot to include the patches for CHANGELOG and API_CHANGES. I updated barh_demo.py as well, and tested the rest of the bar demos. Here's the combined patch against 2515. Sorry for the hassle, I'm a bit new at this. Cheers, Martin John Hunter wrote: >>>>>> "Martin" == Martin Spacek <sc...@ms...> writes: > > Martin> Ah, I was just working off the axes.py file from > Martin> viewcvs. I've checked out now from /trunk/matplotlib/ > Martin> (using tortoise svn). Hopefully this new patch file will > Martin> work. > > Making progress - I was able to apply this and check it in, but I only > got a patch for axes.py (no CHANGELOG, API_CHANGES, modified examples, > etc). Also, examples/barh_demo.py is now broken after application of > this partial patch). Could you get a fresh checkout from svn > (revision 2515) and see what is missing and then provide an updated > patch? > > Thanks! > JDH
>>>>> "Martin" == Martin Spacek <sc...@ms...> writes: Martin> Whoops. Forgot to include the patches for CHANGELOG and Martin> API_CHANGES. I updated barh_demo.py as well, and tested Martin> the rest of the bar demos. Here's the combined patch Martin> against 2515. Martin> Sorry for the hassle, I'm a bit new at this. OK, great. They are in 2516. Thanks! JDH