Add kde support for hist

[gepcel]

Are you instrested in adding a kde support for hist for now?

Should close #794

[codecov]

Codecov Report

❌ Patch coverage is 91.55556% with 19 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ultraplot/internals/inputs.py	71.42%	7 Missing and 3 partials ⚠️
ultraplot/axes/plot.py	86.95%	5 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

[cvanelteren]

This would require a few extra things. First an optional dependency in the install for scipy; a fallback + handling in the code added here and third tests.

[gepcel]

• First an optional dependency in the install for scipy;
• a fallback + handling in the code added here and
• third tests.

3/3 are done.

[cvanelteren]

• First an optional dependency in the install for scipy;

  • a fallback + handling in the code added here and

  • third tests.

2/3 are done. I don't know how to add fallback handling, raise an error if scipy is not installed? And I'm not sure if it's good practise to put one assert in one method or it's ok to put them together.

It's fine if you take this over, or you can leave comments about what else to do, and how to do it.

shouldn't the second bullet be done?

[gepcel]

I don't know what you mean by "fallback handling". Maybe you can explain a little or you can take it over.

[cvanelteren]

What I would do is make the scipy import optional and do a try except block and produce a warning if it is not installed; fallback may have been wrongly stated as this would imply we provide the kde itself

[gepcel]

What I would do is make the scipy import optional and do a try except block and produce a warning if it is not installed; fallback may have been wrongly stated as this would imply we provide the kde itself

Ok, I'll take a look, and add this. I actually did do a research how to calculate kde using pure numpy.

[gepcel]

Now you can review.

[cvanelteren]

I put some notes down for you to look at!

[cvanelteren]

we would need to forward gaussian_kde kwargs too

[cvanelteren]

^this still needs to happen

[gepcel]

done.

[cvanelteren]

actually we may just want to move this block out in helpers somewhere and call it from there

[gepcel]

done

[cvanelteren]

conda does not allow for optional deps we could add this to the pyproject.toml and not bloat conda installs

[gepcel]

Will do.

[cvanelteren]

Ah they were pending my bad!

[gepcel]

The implementation is complete and the code is fairly straightforward. However, I'd like to highlight a few points:

1. In addition to the boolean argument kde that controls whether to plot the KDE line, I added a single **kwargs named kde_kw. This argument accepts two types of parameters: one for generating the KDE line (including stepsize, bw_method, passed to _kde_line1d) and the other for plotting it (all other args passed to ax.plot). I decided against adding separate gaussian_kwargs and line_plotting_kwargs because it would create confusion and unnecessary complexity.

2. There is an import check that raises an error with an explanation if SciPy is not installed. However, there is also another gaussian_kde import inside _apply_ridgeline. I think that part should be dealt with too. And to avoid duplication, the helper function should probably be refactored to serve both use cases.

3. Currently, the KDE functionality only handles univariate data for 1d hist plotting, but it can accept multiple columns to generate multiple kde lines. However, scipy.stats.gaussian_kde supports bivariate data too, so you might consider extending it for hist2d in the future. I have only worked with 1D histogram plotting so far, but I can dig in a little if needed.

4. I added SciPy as a dependency in pyproject.toml under a new optional group called stats. In the future, any additional dependencies for statistical plotting can be placed there.

[cvanelteren]

Feel free to merge

[gepcel]

Feel free to merge

Can I? I don't see the merge option. The refactor codes are too many, I did some simple tests, seems right. You can merge.

Whenever code is refactored to integrate into UltraPlot’s codebase and style, it becomes harder to read. I can only do some tests.