Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add new-style string formatting option and callable option tofmt
inAxes.bar_label()
.#23690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fmt
inAxes.bar_label()
.fmt
inAxes.bar_label()
.story645 commentedAug 21, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think this is totally reasonable given we allow fstrings directly and indirectly for most of the other labeling, so I'm +1 on this. ETA: Also, sorry should have posted this on the issue! |
In general we have not tried to guess which version of format strings the user passed. In the test used here This did come up when we added this in 2019#15602 (comment) and we opted for "old style" at the time. I do not think we should go down the route of guessing which one the user passed (in addition to the two examples above I think our options are
I think option 3 is my favorite followed by 2. My understand is that "fstring" refers to when you do |
story645 commentedAug 21, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think allowing a callable would be great as that would be consistent with the tick formatters and provides the most labeling flexibility, so cool if that's the easiest path forward! ETA 2:@stefmolin you should probably hold off on changing code until more people chime in on what direction this should take. ETA: I do think f-strings should be allowed directly since they're so widely used, but shifting that discussion to#23694 since I don't want that discussion to clobber this one. |
stefmolin commentedAug 21, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@tacaswell - Yes, I was referring to new-style format strings – sorry for any confusion. Going off the other issue, I'm going to update this to accept callables as well.@story645 - I really like the idea of being able to use the tick formatters here as well. |
fmt
inAxes.bar_label()
.fmt
inAxes.bar_label()
.@stefmolin just to be clear, you're 💯% welcome to join in on the formatting discussion where ever it's taking place. |
Uh oh!
There was an error while loading.Please reload this page.
I continue to be very skeptical of trying to guess which format string the user intended. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The format function needs thorough testing.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
It is worth always trying both To@jklymak 's point about being able to force the users preference, I think "use a lambda" is sufficient for that case. |
timhoffm commentedAug 26, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I originally also thought that way, but now think otherwise
Additional note: My only slight concern with prefering %-style is that I would actually prefer {}-style when I had to write that from scratch, because {} is more common nowadays. But I think, that's not important enough to introduce minor backward incompatibilities. |
@timhoffm I'm convinced. @stefmolin What strings were passing the |
@tacaswell - importmatplotlib.pyplotaspltfig,ax=plt.subplots()bar_container=ax.barh(['c','b','a'], [1_000,5_000,7_000])ax.bar_label(bar_container,label_type='center',fmt='{:,.0f}') |
I can reproduce this 🤯 , but I think I have an explanation: If you put a breakpoint in the formatting function you get to:
so I think the problem here is that if the value is a numpy float the string formatting fails to fail. However it is not that numpy floats fully escape formatting: In [16]:'s'%np.float64(5)Out[16]:'s'In [17]:'%d'%np.float64(5)Out[17]:'5' @melissawm have you seen anything like this in numpy before? This seems like a bug, but is rather hard to google... |
Uh oh!
There was an error while loading.Please reload this page.
Ok, thanks pointers from@melissawm and Sebastian, I think I understand what is going on here: Parsing the documnetation on this very carefully:
The input can be:
If we pass a format string with 1 Because Other cases that are surprising: >>>'d'% [1,2,3]'d'>>>'d %(0)d'% [1,2]Traceback (mostrecentcalllast):File"<stdin>",line1,in<module>TypeError:listindicesmustbeintegersorslices,notstr>>>'d %(0)d'% {0:1}Traceback (mostrecentcalllast):File"<stdin>",line1,in<module>KeyError:'0'>>>importnumpyasnp>>>'d %(0)d'%np.float64(5)Traceback (mostrecentcalllast):File"<stdin>",line1,in<module>IndexError:invalidindextoscalarvariable.>>>'d %(0)d'%np.array([1])Traceback (mostrecentcalllast):File"<stdin>",line1,in<module>IndexError:onlyintegers,slices (`:`),ellipsis (`...`),numpy.newaxis (`None`)andintegerorbooleanarraysarevalidindices>>>'d'%np.array([1])'d'>>>'d'% {0:1}'d' The |
Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>
Thanks for the debugging and explanation@tacaswell - very interesting to understand the why behind that issue. |
fmt
inAxes.bar_label()
.fmt
inAxes.bar_label()
.Should I add a what's new entry and/or example for this? |
story645 commentedSep 1, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
yes what's new entry, I think update eitherhttps://matplotlib.org/devdocs/gallery/lines_bars_and_markers/bar_label_demo.html#sphx-glr-gallery-lines-bars-and-markers-bar-label-demo-py orhttps://matplotlib.org/devdocs/gallery/lines_bars_and_markers/barchart.html#sphx-glr-gallery-lines-bars-and-markers-barchart-py with the new formatting? (use callables for one group, strings for the other group maybe?) |
Uh oh!
There was an error while loading.Please reload this page.
@story645 – I added a what's new entry, but I'm thinking it might be better to add the examples from the what's new entry to theBar Label Demo. The demo already has multiple examples, but it feels a bit contrived to change the current ones to use the new functionality since there is really no benefit to doing so (there's currently only one example that even uses For reference, here are the examples I created to make it more apparent why you might use the importmatplotlib.pyplotaspltanimal_names= ['Lion','Gazelle','Cheetah']mph_speed= [50,60,75]fig,ax=plt.subplots()bar_container=ax.bar(animal_names,mph_speed)ax.set(ylabel='speed in MPH',title='Running speeds',ylim=(0,80))ax.bar_label(bar_container,fmt=lambdax:'{:.1f} km/h'.format(x*1.61)) importmatplotlib.pyplotaspltfruit_names= ['Coffee','Salted Caramel','Pistachio']fruit_counts= [4000,2000,7000]fig,ax=plt.subplots()bar_container=ax.bar(fruit_names,fruit_counts)ax.set(ylabel='pints sold',title='Gelato sales by flavor',ylim=(0,8000))ax.bar_label(bar_container,fmt='{:,.0f}') |
IMHO thebar_label demo needs an overhaul anyway. The semanitc content in the existing examples is quite far fetched and doesn't really make sense. I'd like to see more realistic content, like your plots. For now, it's fine to add your plots to the bar label demo as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I still have two minor suggestions, but approve already what is there.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Off-topic:@stefmolin Thanks for working on Matplotlib. I'd like to make you aware of thenew contributor meeting. This is a good place to get to know some of the matplotlib developers, to learn more about the project and of course to ask questions in general. If you are interested, you are very welcome to join. The next date is Tuesday, Sept 6 at 5pm UTC. |
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
@timhoffm - I've incorporated the suggestions and added the two new examples to the demo. Thanks for letting me know about the new contributor meeting – it's during work for me, but I'll see if I can attend 😄 |
Holding off on merging only cause I'm not sure if features are fixed when a release candidate is cut -@QuLogic? |
It is milestoned 3.7, so shouldn't be a problem, right? |
Yeah was just unsure if feature freeze comes in once the RCs are cut or if they can come in - under the wire. Granted, targeting 3.7 gives time to roll out this new formatting support to all the functions with a fmt keyword (and support for formatters) if we want to go that route. |
3.6.0 feature freeze comes in as soon as we have the 3.6.x branch. Then, the PRs are going into main and need to be backported to the 3.6.x brach explicitly. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Addresses#23689
Enhancement to
Axes.bar_label()
to make it possible to format values with f-strings.PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).