feat: adds optional horizontal error bars - #604
Conversation
| cmin: sc.Variable | float | None = None, | ||
| coords: list[str] | None = None, | ||
| errorbars: Literal['band', 'bar', True, False] = True, | ||
| errorbars_x: bool = False, |
There was a problem hiding this comment.
Not sure I like the interface here. It doesn't break existing code, but it feels weird to have a special errorbars_x next to errorbars.
Instead, I would suggest to add more possible values to errorbar: xonly and yonly (or something similar). The behaviour would then be the following:
- if there are both x and y errorbars, show them by default.
- show only y errorbars with
errorbars='yonly' - if
errorbars='bar'is used and we have variances on the coord, just don't show the horizontal errorbars.
We can discuss about the last one, but I think it's probably the most predictable behaviour.
There was a problem hiding this comment.
Hm, I don't think it makes sense to couple them so hard to each other. To me the decision to have errorbars in x versus in y are separate decisions, and I'd prefer to not have one of those decisions coupled to the other.
But I'd be all for changing the name of the argument if you think it is unclear.
There was a problem hiding this comment.
It's not that the name is unclear, it just feels strange to have errorbars and errorbars_x. It makes one think why not errorbars_y? Also, I would vote for showing them by default.
To me the decision to have errorbars in x versus in y are separate decisions
You'd still have the option of turning one or the other off? I'm not sure I get the argument about coupling.
There was a problem hiding this comment.
I'm not sure I get the argument about coupling.
If I have an application with a button that says "turn on 'resolution' errorbars", then it is not enough to make that button alone control the errorbar_x argument. Instead the application has to check the state of the "turn on 'signal' errorbars" button and "turn on 'resolution' errorbars" button together, and based on them both decide what argument to pass when creating the figure.
If I want to change the errorbars in y or in x in isolation I can not just change the value of one flag, I have to first check what the current value is, and based on that determine how to change it.
That's the kind of stuff I mean.
There was a problem hiding this comment.
Also, I would vote for showing them by default.
That would be a quite significant behavioral change :/ and I don't think most people care much about the error bars in x. I think it would make many figures quite "noisy" and that's why I made it off by default.
But sure, we can turn them on by default.
| def __init__( | ||
| self, | ||
| mode: Literal["band", "bar"], | ||
| axis: Literal['x', 'y'], |
There was a problem hiding this comment.
I find the interface here strange, with both mode and axis args.
See other comment about having xonly and ylonly errorbar modes.
I would vote for always showing horizontal errorbars if present by default.
| x, y, yerr=e, color=color, zorder=zorder, fmt="none" | ||
| x, | ||
| y, | ||
| xerr=e if self._axis == 'x' else None, |
There was a problem hiding this comment.
I don't think errorbars in x would work well here? We are using the center of the bins for display purposes, but it's a bit like cheating.
The variances are actually on the bin edges, meaning there is uncertainty about where the edges are located (which is conceptually weird).
I think I would just drop horizontal errorbars for plotting bin-edge data?
There was a problem hiding this comment.
Agreed 👍
I think I would just drop horizontal errorbars for plotting bin-edge data?
Sounds good to me. We can always add support once we know how that case can be handled cleanly.
2 participants
Fixes #295