Histogram breaks

[bbeat2782]

Describe your changes

Implemented --breaks/-B to ggplot histogram.

breaks demonstration.zip This notebook contains breaks demonstration and errors I implemented.

Issue number

Closes #719

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--767.org.readthedocs.build/en/767/

[bbeat2782]

breaks demonstration.zip

This is the implemented behavior of breaks. @edublancas Could you take a look at the demonstration and check whether it behaves as we want when you have time?

Since I have two other issues, this does not need an immediate check.

[edublancas]

looks great! don't forget to add some reference images and test cases

[edublancas]

I think you can use cmd to check if both arguments exist.

e.g.,

if cmd.args.bins and cmd.args.breaks:
    raise UsageError(...)

let me know if that works, otherwise we should fix it since validating arguments with regular expressions is going to be a pain if we have more complex combinations

[bbeat2782]

Because of the default value of bins (which is 50 for the magic command), cmd.args.bins always return an integer value. Thus, I used the following code instead.

bin_specified = " --bins " in line or " -b " in line
breaks_specified = " --breaks " in line or " -B " in line
if bin_specified and breaks_specified:
     raise exceptions.UsageError(...)

[tonykploomber]

LG, very comprehensive test case covered, GJ

[neelasha23]

Would it make sense to allow users to pass a range of values?

points = list(range(1000, 6000, 500))
%sqlplot histogram --table penguins.csv --column body_mass_g --breaks {{points}}

@edublancas @bbeat2782

A few more observations:

1. I'm not able to understand this result: If we specify %sqlplot histogram --table penguins.csv --column body_mass_g --bins 100 the plot is as below (x value goes upto 6000+)

But when we specify breaks where last break is less than highest x value it doesn't show data beyond the last point:

Shouldn't there be one more bin after 5000 till the highest value of body_mass_g ?

2. UsageError: All break points are lower than the min data point. : It might help the user if we specify what the minimum point is (similar for max data point).

3. There would be only 1 bin if first point is less than lower datapoint and second point is higher than max value. We can provide some info to guide user when specifying points:

[yafimvo]

Looks good.

Added some comments.

I think we should add some integration tests and make sure it works (or doesn't) on other databases (PostgreSQL, questdb, etc...)

[yafimvo]

This error repeats itself. Can we delete the error handler from here?

Also, please change ValueError to exceptions.ValueError

[bbeat2782]

Deleted the error handler in magic_plot and changed ValueError to exceptions.ValueError

[bbeat2782]

@neelasha23

allow users to pass a range of values

With this PR implementation, the following works.

points = " ".join(map(str, range(1000, 6000, 500)))
%sqlplot histogram --table penguins.csv --column body_mass_g --breaks {points}

Shouldn't there be one more bin after 5000 till the highest value of body_mass_g ?

1. When specifying breaks, we are specifying a starting point, break points, and ending point. Thus, the data we query is limited to starting point <= data <= ending point. That is why the upper limit is 5000 here. This range limit is mimicking ggplot behavior in R. To avoid confusion, I added an explanation of breaks limiting range in magic-plot.md.

It might help the user if we specify what the minimum point is

2. Changed the error message to UsageError: All break points are lower than the min data point of {min point}. (similar to the max data point)

There would be only 1 bin if first point is less than lower datapoint and second point is higher than max value. We can provide some info to guide user when specifying points:

3. For this, do you think displaying a message will work better or giving a general info on the docs will work better?

[bbeat2782]

@yafimvo

I think we should add some integration tests and make sure it works (or doesn't) on other databases (PostgreSQL, questdb, etc...)

Added test cases and images in test_questDB.py.

[edublancas]

@neelasha23 @yafimvo please review again

[neelasha23]

3. For this, do you think displaying a message will work better or giving a general info on the docs will work better?

Docs should be fine

[neelasha23]

Looks good!

[edublancas]

looks good, one thing is missing is adding one new test here:

jupysql/src/tests/integration/test_generic_db_operations.py

Line 295 in b6f07dc

"histogram-bins",

you can just add an example that runs using --breaks, let's see if it passes. if you find compatibilities, ping me and we can decide whether we support it or not. my guess is that it'll work with the databases that we're testing there

[bbeat2782]

one thing is missing is adding one new test here:

@edublancas I added histogram-breaks, and it passed all tests

[edublancas]

awesome!