| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I'm a bit confused by this taking the data row-wise. So for consistency I would suggest to do the same here as well. |
Sorry, something went wrong.
|
I did it the way it is because I think most users will have their data for each bar in an series of arrays and would like to just put them into one bar call. The post referenced in the feature request (https://stackoverflow.com/questions/14270391/python-matplotlib-multiple-bars?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa) does it that way so thats what I went with. I can change it if that's what you want but I'd like to be sure that others agree with you about it first. |
Sorry, something went wrong.
|
I'd need to double check, but I somewhat suspect you broke the API for colors etc as well. Or at the very least, you need to document the changes. |
Sorry, something went wrong.
|
In terms of plotting just a single bar graph, nothing is changed. I could, however put in new documentation describing how optional parameters are treated by the multibar code. |
Sorry, something went wrong.
|
I think there's a pretty bad issue right now of rows vs columns in this kind of data (see e.g. #8092) and would suggest we figure out the best way to handle that first (my opinion is in the linked issue, but mostly I just want things to be consistent :-)) before making the situation even more tangled... |
Sorry, something went wrong.
|
A cultural standard is to organize data in tables. Those tables are read column by column. Example (from the famous work by Jean Baptiste Perrin about Brownian Motion) While of couse equally possible, I rarely see anyone creating such table by aranging the independent data horizontally. Example from Spreadsheet software: More specifically, reading in data into numpy from such table with n rows and m columns, e.g. via loadtxt, creates a numpy array of shape (n,m), so we get the data of the each table column along the first dimension of the numpy array. Equally pandas uses the concept of columns containing data from a series. To be honest I'm at loss as to why one would suddenly use the transpose of that. (I know that matplotlib already has this transposed concept e.g. in the case of asymmetrical errorbars, where data needs to be row-wise, but that is surely confusing.) |
Sorry, something went wrong.
|
Ok. I made it take the data column-wise. |
Sorry, something went wrong.
| height : scalar or sequence of scalars | ||
| height : scalar or array-like of shape(N,) or shape(2,N) | ||
| The height(s) of the bars. | ||
| 2-dimensional arrays represent grouped bars. |
There was a problem hiding this comment.
I think this should be
scalar or array-like of shape(N,) or shape(N,M)
right? (same below for barh)
And the role of N and M should be explained here. Something like
..., where N is the number of bar positions (same as the length of x) and M the number of groups.
Sorry, something went wrong.
Data StructureI think this is good. It is consistent with how plot and hist etc. handle 2D numpy arrays; and as outlined above this is in my optinion the correct way anyways. I do see the problem mentionned by @anntzer and discussed in #8092 for boxplots. I would argue that in the case of bar plots, there is no need to even support unequal length datasets (since you never acually want to leave out the last bars, and hence need to use np.nan for missing bars anyways, leading to nice 2D arrays.) DocumentationA minor point (but not negligible) is the example:
(I think I slightly prefer the first option, simply because it keeps the number of different gallery examples low. But maybe it's also good to show an example with more than two groups?) Finally, I think this grouping mostly makes sense for categorical data. Since matplotlib supports categorical data in the sense of strings supplied to plotting function, wouldn't it be nice to have this incoorporated in the example? So using ax.bar(labels, heights) instead of ax.bar(x, heights, label=labels). |
Sorry, something went wrong.
|
While I understand the motivation, I have several concerns about this this. If this goes in I suspect we will almost immediately get a request for this, but stacked. The obvious way to add this is to add some grouped-only relevant kwargs which is something we diffidently want to avoid. I am also concerned about the change in return type based on the type of the input. The isinstance checks are troubling, we try very hard to avoid those. Is this always the broadcast semantic that we want? I think this will lead to a case where you can specify height column-wise, but row-wise for everyhing else. How does this play with datastructures that are easy to get out of pandas dataframes? I think this may be better as a new top-level function to avoid the variable return types, easily support the stacked case, and to (without concern for back compatibility) deal with the broadcasting of all the parameters issues. |
Sorry, something went wrong.
With the function now taking the data in columns, this plays very well with pandas. As stated above this is one of the motivations to use columns instead of rows. If df is a dataframe, plt.bar(df.index, df.values) It seems though that it does not allow for the data argument to be provided. I guess this could be added, and take array-like arguments, like plt.bar("x", ["y1", "y2"], colors=["colorcolumn1","colorcolumn2"], data=df); is that what is meant here? I cannot say much about the other points - probably they are all valid. |
Sorry, something went wrong.
|
If you guys could give me a checklist with what you want me to do I would be happy to oblige. |
Sorry, something went wrong.
|
We talked about this on the weekly call and the consensus is that this is a very useful functionality, but the front end complexity people are going to want is quite large and going to rapidly grow (give a mouse a cookie....) and adding a large branch that recursively calls back into it's self is too much complexity on the implementation side. The route we would like to see this take is to create a new function (multi_bar or something, (I'm really bad at names)) that is only for multi bar (both stacked and adjacent). As mentioned above this gets around almost all of the back-compatibility issues and shoe-horning the multi-bar API into the existing bar API. A good measure if the API is flexible enough is if we can replace the multi-dataset plotting in hist with a call to this function. What kwargs from bar do we want to carry over? bottom and align seem like candidates of things to drop, what else can go? What are the desired semantics on:
Is it worth making a new Artist class for this so that users can change from stacked to adjacent interactively? I think the next step is to write out the docstring for what this function! |
Sorry, something went wrong.
There was a problem hiding this comment.
See comments for details
tldr; make this a separate function
Sorry, something went wrong.
|
Still thinking of working on this one @UTSC-SISYPHUS ? |
Sorry, something went wrong.
|
Sorry, I had exams. I will still work on it. I'll get to it this weekend. I could do with a bit of information on the dataframe @ImportanceOfBeingErnest mentioned for the puprpose of supporting the data kwarg though. |
Sorry, something went wrong.
|
Added documentation for new function. Please give me feedback. |
Sorry, something went wrong.
|
It looks like this currently only consists of documentation semantics. The code is simply pass. |
Sorry, something went wrong.
|
I had this working in another function but changes were requested. I'm asking if I should write the code for the documentation as it is now or if I should make some more changes first. |
Sorry, something went wrong.
|
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
Sorry, something went wrong.
|
Superseded by #24313. Please move further discussion there. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
Added the ability to give a 2D matrix to pyplot.bar to plot multiple bar graphs with one call.
Example :
will create

#10610
PR Checklist