%sqlplot bar and pie charts

[mehtamohit013]

Describe your changes

Added bar and pie chart to sqlplot

Issue number

Closes #417

Checklist before requesting a review

• Performed a self-review of my code
• Formatted my code with pkgmt format
• Added tests (when necessary).
• Added docstring documentation and update the changelog (when needed)

---

📚 Documentation preview 📚: https://jupysql--508.org.readthedocs.build/en/508/

[edublancas]

hey @jorisroovers, wanna take a look at this and share your feedback?

you can see an interactive demo here: https://binder.ploomber.io/v2/gh/mehtamohit013/jupysql/plot?urlpath=lab/tree/doc/api/magic-plot.md (scroll down to see the bar and pie chart examples)

[yafimvo]

@mehtamohit013
added some comments.
also, please rebase.

[jorisroovers]

hey @jorisroovers, wanna take a look at this and share your feedback?

Looking pretty good. Some feedback from spending a few mins...

1. Pie chart: Would be good if there was an option to show the values (and percentages) inside or parts of the pie chart (or next to it with legend, probably easier to get right with edge cases).

2. Pie chart: I would show the pie pieces in order of the query result (i.e. let the user decide) and not always order by size.

3. Bar chart: x label looks off

4. Bar chart: Also would be good to show exact number of each bar (inside bar) or in (optional) legend.

[mehtamohit013]

@edublancas

• For 1 and 4 added an additional argument -sn/--show_num to display the number and percentage
• Fixed 3

You can test out the above changes on binder

• Can you please elaborate on the 2nd point, I don't quite get it?

[mehtamohit013]

Waiting for #486 to merge

[edublancas]

can we add support for NULL values? you can run a query to filter them first

[mehtamohit013]

Should I raise some warning when there is NULL?

[edublancas]

good point. not a warnings.warn, but printing a message saying that we're removing NULLs makes sense

[mehtamohit013]

Should the message be printed only when there is NULL or a generic message?
Currently added a general message:

print(f"Removing NULLs, if there exists any from {column}")

[edublancas]

i didn't think about this. we have two options

1. check if there is a null (by querying the table). if so, show a message and filter them out
2. embed the null filter in the existing SQL queries as a subquery - this will impact performance but I'm guessing most SQL engines are smart enough to just pass the data once

let's implement 2. this implies that we won't show any message to the user since we'll filter out NULLs all the time - so there's no point in showing a message. we can later optimize.

[mehtamohit013]

@edublancas
Implemented 2 option + Generic null Message for info :

Sample SQL query:

select "{{x_}}" as x,
"{{height_}}" as height
from "{{table}}"
where "{{x_}}" is not null
and "{{height_}}" is not null;

[yafimvo]

Waiting for #486 to merge

@mehtamohit013 We shouldn't wait for the merge.

Also, I see you addressed some of the issues. Please mark the issues you have fixed as "resolved" so that it will be easier to review them again.

P.S please rebase

[mehtamohit013]

@yafimvo @edublancas
While running matplotlib image tests, they are passing locally, but not in Github actions, as they are generating a different image in GitHub actions, but I make sure I have the same version locally as well on Github actions i.e. 3.7.1

Any idea why this is happening?

[edublancas]

@yafimvo @edublancas While running matplotlib image tests, they are passing locally, but not in Github actions, as they are generating a different image in GitHub actions, but I make sure I have the same version locally as well on Github actions i.e. 3.7.1

Any idea why this is happening?

this has happened before. usually fonts are the problem (even if you have the same matplotlib version, the fonts might be different).

there's an option to ignore text while testing, if this doesn't fix the issue, it'll be a bit challenging to debug because we don't have access to the failed images (I just opened this: #543)

see if you can quickly fix this, @mehtamohit013. if not, let me know and we can block this, you can work on #543 and then we can resume this work

[yafimvo]

@mehtamohit013 Try to use these images

test_magic_plot.zip

[mehtamohit013]

Try to use these images

Thanks, @yafimvo, matplotlib tests are now working.
Btw, how do you get theses images?

[mehtamohit013]

@edublancas
Ready for merge

[yafimvo]

Thanks, @yafimvo, matplotlib tests are now working.
Btw, how do you get theses images?

@mehtamohit013 The images failed due to minor font-weight differences so I just created new base images with the expected font-weights.

[edublancas]

this is almost ready. just implement the NULL support and show a message to the user

[edublancas]

address the NULLs comment and add a unit test with some dataset that has NULLs

[mehtamohit013]

@edublancas
Working on tests. Please don't review
Accidentally asked for a review. There is no such button as take review requests back 😅

[mehtamohit013]

@edublancas
Ready for merge

[edublancas]

awesome work!