Repository navigation
A reasonable default width for bar() with units? #13236
Description
In #12903 we merged a PR fixing the handling of the default width (0.8) when bar (barh, broken_barh) is called with unitized data (I was among the PR approvers; see also #13187 as followup). Briefly, the idea is to support both unitless and unitized widths, so that one can do
bar([dates]) # default width: 0.8 bar([dates], width=[timedeltas]) # or single timedelta bar([categoricals]) # default width: 0.8 bar([categoricals], width=[unitless]) # or single scalar
The main trickiness is that the default width is unitless (0.8), but one can also pass a unitful width, so it's not clear how to add it to the unitized x; moreover, note that the unit of x can either correctly handle addition (time + timedelta), or not (categoricals, which need to go through the converter first). So the end design is something like
try:
dx = deunitize(orig_x + orig_dx) - deunitize(orig_x)
except ...:
dx = deunitize(orig_dx)
where orig_x and orig_dx are the user inputs, either unitized or deunitized.
However, I now feel like this is not really looking at the problem under the right angle: if the user input is unitized, what's the chance that 0.8 "deunitized-units" is a reasonable default width? It is rather low, I would guess. Instead, it would seem that the intent of the default would/should be "0.8 times the spacing between the bars" (in the common case of evenly spaced bars).
Hence, a solution may perhaps be to change the interpretation of a unitless width in the case of unitized data, to have it mean (e.g.) a fraction of the (mean) spacing between the bars (i.e. (max(x) - min(x)) / (n - 1)). Note that because bar did not support unitized data up to now, there are no backcompat concerns.
Mostly @jklymak I guess. Allowing myself to mark this as release critical mostly so that we don't have to go through a painful deprecation period if we agree on this, but feel free to untag.
Activity
jklymak commented on Jan 20, 2019
I think this is reasonable, but a bit mysterious to only apply to unitized data.
Doesn’t obviate the need for the other PRs, but may simplify them somewhat.
timhoffm commented on Jan 20, 2019
Good catch, A fixed "deunitized default" does not make sense. However, the topic is a bit complex.
IMHO the unit-free numeric values corresponding to unitized data is semi-public. For simple cases, people should not have to know about it (i.e. plotting bars based on categoricals or dates). However, we cannot completely hide it, because people might want to draw additional elements to the Axes and thus might need to know about the actual data-space of the Axes.
Some general thoughts:
jklymak commented on Jan 20, 2019
The paradigm for units is to allow the converter to decide if it likes the non-unitized float and what to do with it.
anntzer commented on Jan 20, 2019
The problem of having "auto" mean 0.8*(the mean spacing) for unitized data but otherwise requiring unitized widths for unitized data is that this does not allow adjusting the bar widths for categorical data, because for those you can really only do the computation afer deunitizing (an my guess is that quite a few people want to use width=1 in that case).
Hence the proposal to break the symmetry between unitless data (width is absolute) and unitized data (width is absolute if unitized, relative if not). It's not optimal, but so far no-one was using bar() with unitized data (as that did not work) so hopefully users will learn about that (and if they find this too confusing, we can always revisit the design).
I am definitely not proposing to change the interpretation of width for unitless data.
anntzer commented on Jan 21, 2019
See #13254 for an implementation of the proposal.
dopplershift commented on Jan 24, 2019
I see the appeal of using a relative meaning for width, but I'm really not wild about the idea that for this one corner (call it 25% of options) it means relative width, but for the rest of cases it means absolute. This kind of guessing is what usually gets us into long term maintenance troubles.
While I get purity arguments here, do we have a test case that actually shows problematic behavior in the current implementation WRT units and bar width? I get it might be suboptimal, but is it bone-headedly stupid?
jklymak commented on Jan 24, 2019
You can’t do datetime + float. It fails right now on 3.0.2 unless you specify width in timedelta
jklymak commented on Jan 24, 2019
Btw I agree that if we change to a relative default for some cases it should be relative for all cases.
anntzer commented on Jan 24, 2019
The current default (as of master) is basically a bar of 0.8 days, because datetimes are converted to scalars on a 1-day scale. So you'll get something reasonably nice looking if your x's are ~1 day apart, but not if they're ~1y apart. For scalars, I guess that's typically not a problem because most bar plots effectively use the x-axis as ordinals (and thus consecutive integers)?
I guess the alternative is to document the default as 0.8-of-whatever-your-units-convert-to-unitless, but isn't the point of unit systems that you can't, well, do non-homogeneous operations?
dopplershift commented on Jan 24, 2019
Well, unit support enables doing the same operation across values given with different scaling/offsets. What you're proposing is doing a completely different operation by default. IIUC:
While it could make for better default plotting out of the box, it means some users will be confused by the change in the plot when they switch from plotting on a date time axis to one on plain floats. I'm not full out against it, but it does make me pause. And it does seem to be against the general grain of matplotlib development later where we're moving away from such magical, polymorphic behavior.
anntzer commented on Jan 24, 2019
I guess I just have to accept I fundamentally don't understand what is the general philosophy behind unit support in Matplotlib (we already had this debate some time ago and frankly, every time I try to understand it, I fail... no need to relitigate the whole thing here); in the meantime, just documenting the actual behavior (0.8-of-whatever-your-units-converts-to) seems good enough.
23 remaining items
jklymak commented on Sep 14, 2019
You should be able to set the width with a timedelta or Timedelta64. It’s not trivial to explain how bar widths interact with all the units If you had a suggestion for where it would have been helpful to see this in the docs that would be very helpful
ImportanceOfBeingErnest commented on Sep 14, 2019
An advantage of the currently implemented way is that it's totally deterministic. Units are always data units. And width is 0.8 data units, unless changed by the user. The same is actually true for pandas (though it might be 0.6 instead of 0.8), but the confusing bit with pandas is that barplot units are categorical: each bar is 1 unit away from the next, independent on what the formatter says. In that sense pandas plots might result in completely confusing output when an unequally spaced timeseries is plotted, such that bars are equal in width in display space, but look unequal in width in data space because the formatter suggests different axis units.
jklymak commented on Sep 14, 2019
In master, after #12903 the current default is a bare 0.8 float with no unit information, not 0.8 in "data units" (I don't think it was ever 0.8 in "data units").
If x0 has units, and x = convert(x0) is the float representation, and dx0 is the width, then we try converting dx0 using dx = convert(x0+dx0)-x before just doing convert(dx). We do this to allow unit-aware dx0 and to allow units that have __add__ defined between them (like datetime and timedelta in particular) work as widths.
If anyone wants to have a unitized bar that does something else, i.e. x0 = myUnitPacker(5, 'gallon') and dx0 = myUnitPacker(2, 'liter') and they have defined __add__ between these two units, then this width will now work. With the current default on 0.8, it would be up to the unit converter to decide what 0.8 means. Some converters pass through unitless data, some do not.
ImportanceOfBeingErnest commented on Sep 14, 2019
We call the units of the axis scale "data" throughout, e.g. ax.transData.
jklymak commented on Sep 14, 2019
That is confusing though, because an axis has "units" that get converted by the axis' unit converter to floats. I believe what you are referring to are data co-ordinates.
ImportanceOfBeingErnest commented on Sep 14, 2019
I'm no native speaker, but I believe you would call the distance between 2 and 1 on an axis "1 (data) unit" not "1 (data) coordinate"?
jklymak commented on Sep 15, 2019
Just to clarify: The problem with our units handling as pointed out by @snowzky, is that at the moment is that the user has no way to know what "0.8" means relative to the units they are plotting in. i.e. imagine if units were lengths, and they passed in bar(myUnits([0.4, 1.2, 0.7], 'meters'), [12, 11, 19]) they have no way of knowing what the unit converter does to the default 0.8. Is 0.8 translated assuming that it is 'meters'? Or maybe kilometres (in which case the default bars will be far too thick), or maybe micrometers, in which case the default bars are much too thin. In the case of datetime objects, 0.8 days makes sense, but so would 0.8 seconds. For some units, it is possible that convert(0.8) will even error out and bar will fail with the default width.
I'm not sure what the way forward is. Making the width the median distance between x values is tempting, but it is also impossible to report that width (with appropriate units) back to the user because we lose that information immediately upon conversion. We have been talking about a new data framework where the raw x and dx are retained as well as the converted versions. That would be a big change, but something like that is on the roadmap for 4.x
timhoffm commented on Sep 15, 2019
I'm tempted to go with a new default 'auto', which is 0.8 median width.
I assume that all use-cases that use the current default and that would be affected by the proposed change are more or less heavily broken with too small or broad bars. Breaking them is a penalty we may consider bearable.
github-actions commented on May 30, 2023
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!
kylrth commented on May 30, 2023
Thanks to the ping from the bot, I tested my example from above today with version 3.7.1:
Are we satisfied with this? The bars are visible now, which means people like me who are confused why their plot is blank will have a better idea of what's happening. But obviously it's an unusable default.
jklymak commented on May 30, 2023
I thon @timhoffm 'auto' proposal is a good one. Someone just needs to implement it in a reasonable way.
[allow] ratio inputs for `(h)bar.width` (#118)