feat(python): add plot namespace (which defers to hvplot) #13238

MarcoGorelli · 2023-12-24T10:38:44Z

Much simpler version of #13149

The idea to just defer to hvplot (much like how read_database_uri, as far as I understand, defers to connectorx)

Demo: (note: these plots are interactive, I took a screenshot with my mouse over one of them to show the popup that appears)

Doing .collect within a plotting function sounds potentially like a footgun, and a risk that people re-trigger the same expensive IO function over and over again? I'd suggest, at least to start, to only have .plot on DataFrame, so people are expected to collect before plotting

MarcoGorelli · 2023-12-24T11:00:17Z

@hoxbro @maximlt @jbednar @MarcSkovMadsen I think you're all involved in hvplot (apologies if not - your names are ones I've come across in related discussions)? If so, would be curious to hear your thoughts on this, whether it seems maintainable, and if there's any risks Polars should be aware of.

hvplot has been friendly and welcoming to Polars so I hope this could take the collaboration further 🤝

(I'm aware it's xmas eve, so sorry for writing today - I don't expect a reply, I've just got a long train ride to visit family and so figured I'd try this out)

ritchie46 · 2023-12-24T11:51:24Z

For some people, Christmas comes early this year.

MarcSkovMadsen · 2023-12-24T18:52:05Z

But if plotting requires running some operations for example if using 'by' argument, making boxplot, histogram or using datashader, would you not want the plotting library to control when things are collected?

(I'm not the expert)

MarcoGorelli · 2023-12-24T21:55:13Z

I'd suggest moving incrementally:

start with DataFrame.plot (based on comments/reactions so far, fairly uncontroversial?)
then, discuss LazyFrame.plot later, as I think that's more complicated (see semi/anti join #5508 (comment) for an example of how it's possible to collect-shoot yourself in the foot). I'd like to see at least some discussion of collect-footguns in the context of plotting before adding LazyFrame.plot. But let's save that for a future PR / discussion if it's OK

ritchie46 · 2023-12-25T09:01:53Z

then, discuss LazyFrame.plot later

Yes.. that's a big no-no for me.

jbednar · 2023-12-27T01:27:00Z

This sounds great! I can't think of any risks for doing this; hvPlot is part of our daily workflows for Pandas and Dask DataFrames already, and I'm very pleased for Polars to be joining this list. I don't have experience with using Polars myself, but .collect sounds like it might be conceptually similar to Dask .compute()? For Dask we keep it lazy all the way through to plotting so that tools like Datashader can do the actual rendering in a fully distributed fashion, with chunks of each plot rendered to an image or array in parallel and only the final rendered chunk needing to cross the wire out of each remote worker. Datashader doesn't currently support Polars, but I'd hope it will some day, and so it would be good to choose an approach here that anticipates having that work smoothly.

MarcoGorelli · 2023-12-27T15:29:32Z

Datashader doesn't currently support Polars, but I'd hope it will some day, and so it would be good to choose an approach here that anticipates having that work smoothly.

Hope so too! Looks like there's already an open issue holoviz/datashader#1199

Adding DataFrame.plot now doesn't preclude adding some LazyFrame.plot solution later on, possibly with some safeguards (e.g. a "I know what I'm doing" kind of flag)

In the meantime, I think this would already be an improvement:

the plots look good by default
as far as I can tell, hvplot is a well-maintained project
users would easily be able to discover how to produce high-quality plots during interactive development

It's fairly common to hear user Polars users say they wish they could quickly add .plot to the end of a cell to see how something looks, like they did with pandas. Except this would be a lot better, because of how good hvplot is!

I'll mark as ready-for-review once CI is green

stinodego

The structure here looks great!

I only wonder where the limits are - are all data types supported, for example? What happens if we want to plot some dataframes with Decimal columns? What kind of error do I get if I try to plot an empty DataFrame / the column I am trying to plot is missing?

I would appreciate some additional tests to showcase this.

py-polars/tests/unit/namespaces/test_plot.py

py-polars/polars/dataframe/frame.py

MarcoGorelli · 2023-12-28T16:24:18Z

py-polars/polars/utils/various.py

@@ -392,7 +392,7 @@ def __get__(  # type: ignore[override]
            return self.fget(  # type: ignore[misc]
                instance if isinstance(instance, cls) else cls
            )
-        except AttributeError:
+        except (AttributeError, ImportError):


adding this to avoid an importerror if building the docs without hvplot installed. I don't think hvplot should be required for building docs, and it's probably not worth showing plots in the docs themselves

stinodego

This looks good - I am just a bit unsure about relying on the library internals like this. Feels like this would break easily when new hvplot versions are released. It would be nice to get some feedback from the hvplot people.

However, I think this version is probably good enough to be released and get some user feedback. It's probably stable enough on the short term and we can improve as we go along.

py-polars/polars/dataframe/frame.py

stinodego · 2023-12-31T02:45:01Z

py-polars/polars/series/utils.py

+            # private
+            not name.startswith("_")
+            # `.plot` not available on Expr
+            and name != "plot"


This shouldn't be required - plot is not an empty function so it's skipped. Everything runs fine when I comment this out - am I missing something?

did you try building the docs?

Hm no, indeed it fails on building the docs. But the problem isn't that .plot is not available on Expr. The problem is that it calls hvPlotTabularPolars which imports polars, which doesn't exist while building the docs.

I'm not sure of the best solution here. I would ideally want to avoid hardcoding random stuff here as the functionality is already complex enough. But I'm not sure what the best solution would be 🤔

I think we should probably update sphinx_accessor in some way to avoid this from happening. Looking into it as we speak...

True, but my understanding is that expr_dispatch is only needed for methods available on Expr

you're right about complexity, I just couldn't think of a solution which doesn't add any. maybe there's a more generic solution which would mean that this isn't necessary

cc @alexander-beedie in case you have ideas here

hoxbro · 2023-12-31T07:40:23Z

I have answered the questions and the changes LGTM.

I would make one suggestion of adding a plot accessor to the LazyFrame, with an error or warning which says plotting is not supported for LazyFrame, please call .collect() beforehand.

MarcoGorelli · 2023-12-31T08:01:35Z

Thanks for your reviews!

Sure, I've switched to using hvplot.post_patch - that does indeed look better! Glad you're open to a "sister PR" in hvPlot, I'll mark holoviz/hvplot#1243 as ready-for-review if/when this is accepted after the release

I'm not sure about adding a function which only raises for LazyFrame - any function which is defined is going to tab-complete for users, and if something only raises then I think I'd prefer to keep it out of the API (e.g. pivot is only defined for DataFrame)

hoxbro · 2023-12-31T08:28:49Z

I'm not sure about adding a function which only raises for LazyFrame - any function which is defined is going to tab-complete for users, and if something only raises then I think I'd prefer to keep it out of the API (e.g. pivot is only defined for DataFrame)

That seems like a fair point. My thought was more helping users. An example could be if they started with a DataFrame, did some work, and plotted, and after that, they changed it to a LazyFrame, they will get a non-obvious AttributeError. I don't know if that is too niche of an example. For the autocomplete, at least in IPython/Jupyter, it is possible to exclude it with the __dir__ method.

It was only a suggestion and I trust your judgment 🙂

py-polars/polars/dependencies.py

py-polars/pyproject.toml

stinodego

There's just the one loose end around the interaction between Sphinx and expr_dispatch. We can try to solve it now or keep it as a TODO. Otherwise, this looks good and I think it's a great addition!

ritchie46 · 2023-12-31T11:18:06Z

Al right! Let's get this in. Thanks a lot @MarcoGorelli! Curious how this is received.

alonme · 2024-01-02T15:48:46Z

Might be worth adding Polars in this section
https://github.com/holoviz/hvplot?tab=readme-ov-file#hvplot-works-with-the-tools-you-know-and-love

MarcoGorelli · 2024-01-02T16:18:40Z

can you add this an issue to hvplot please?

MarcSkovMadsen · 2024-01-02T16:30:39Z

can you add this an issue to hvplot please?

Done. Please verify this is what you suggest @alonme holoviz/hvplot#1244

MarcoGorelli force-pushed the hvplot-backend branch 2 times, most recently from 203b007 to fe06c3a Compare December 24, 2023 10:43

add plot namespace which defers to hvplot

2619c8a

MarcoGorelli force-pushed the hvplot-backend branch from fe06c3a to 2619c8a Compare December 24, 2023 10:45

MarcoGorelli mentioned this pull request Dec 24, 2023

RFC: feat(python): add plotting backend #13149

Closed

MarcoGorelli mentioned this pull request Dec 24, 2023

Document polars in reference guide? holoviz/hvplot#1237

Closed

MarcoGorelli added 3 commits December 27, 2023 16:11

plotting -> plot

4805791

Merge remote-tracking branch 'upstream/main' into hvplot-backend

648c16f

lint

fe8729d

MarcoGorelli marked this pull request as ready for review December 27, 2023 15:42

MarcoGorelli requested review from ritchie46, c-peters, stinodego and alexander-beedie as code owners December 27, 2023 15:42

MarcoGorelli changed the title ~~RFC feat(python): add plot namespace (which defers to hvplot)~~ feat(python): add plot namespace (which defers to hvplot) Dec 27, 2023

github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars labels Dec 27, 2023

stinodego requested changes Dec 27, 2023

View reviewed changes

py-polars/tests/unit/namespaces/test_plot.py Outdated Show resolved Hide resolved

py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved

py-polars/polars/dataframe/frame.py Outdated Show resolved Hide resolved

MarcoGorelli added 3 commits December 28, 2023 10:59

Merge remote-tracking branch 'upstream/main' into hvplot-backend

4123a73

add tests for series.hist and unsupported dtype

ff7aaa6

add Series.plot to docs

019bcb9

MarcoGorelli marked this pull request as draft December 28, 2023 12:28

Merge remote-tracking branch 'upstream/main' into hvplot-backend

0506a16

MarcoGorelli added 2 commits December 28, 2023 15:56

fix docs build

af60d9a

no need to require hvplot for docs

28865de

MarcoGorelli commented Dec 28, 2023

View reviewed changes

skip plot doctest

c3ab5e6

MarcoGorelli marked this pull request as ready for review December 28, 2023 16:50

MarcoGorelli requested a review from orlp as a code owner December 28, 2023 16:50

MarcoGorelli requested a review from stinodego December 28, 2023 16:54

MarcoGorelli added 3 commits December 29, 2023 18:46

Merge remote-tracking branch 'upstream/main' into hvplot-backend

e0af814

raise if hvplot not installed or isnt >=0.9.1

56612b3

Merge remote-tracking branch 'upstream/main' into hvplot-backend

0a4ae04

MarcoGorelli mentioned this pull request Dec 30, 2023

add downstream polars test holoviz/hvplot#1243

Merged

stinodego requested changes Dec 31, 2023

View reviewed changes

use hvplot post_patch

3696df4

lint

ab84eb1

MarcoGorelli mentioned this pull request Dec 31, 2023

Discuss: LazyFrame.plot? #13339

Open

stinodego reviewed Dec 31, 2023

View reviewed changes

py-polars/polars/dependencies.py Outdated Show resolved Hide resolved

MarcoGorelli added 3 commits December 31, 2023 09:29

simplify, remove holoviews from dependencies.py

19aca75

remove final holoviews

30e4e01

link to PR comment, add TODO

bba5c5a

stinodego reviewed Dec 31, 2023

View reviewed changes

py-polars/pyproject.toml Outdated Show resolved Hide resolved

stinodego approved these changes Dec 31, 2023

View reviewed changes

ritchie46 merged commit 2341372 into pola-rs:main Dec 31, 2023
17 checks passed

stinodego added the highlight Highlight this PR in the changelog label Dec 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(python): add plot namespace (which defers to hvplot) #13238

feat(python): add plot namespace (which defers to hvplot) #13238

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

ritchie46 commented Dec 24, 2023

MarcSkovMadsen commented Dec 24, 2023

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

ritchie46 commented Dec 25, 2023

jbednar commented Dec 27, 2023

MarcoGorelli commented Dec 27, 2023 •

edited

Loading

stinodego left a comment

MarcoGorelli Dec 28, 2023

stinodego left a comment

stinodego Dec 31, 2023

MarcoGorelli Dec 31, 2023

stinodego Dec 31, 2023 •

edited

Loading

MarcoGorelli Dec 31, 2023

hoxbro commented Dec 31, 2023 •

edited

Loading

MarcoGorelli commented Dec 31, 2023

hoxbro commented Dec 31, 2023

stinodego left a comment

ritchie46 commented Dec 31, 2023

alonme commented Jan 2, 2024

MarcoGorelli commented Jan 2, 2024

MarcSkovMadsen commented Jan 2, 2024 •

edited

Loading

feat(python): add plot namespace (which defers to hvplot) #13238

feat(python): add plot namespace (which defers to hvplot) #13238

Conversation

MarcoGorelli commented Dec 24, 2023 • edited Loading

MarcoGorelli commented Dec 24, 2023 • edited Loading

ritchie46 commented Dec 24, 2023

MarcSkovMadsen commented Dec 24, 2023

MarcoGorelli commented Dec 24, 2023 • edited Loading

ritchie46 commented Dec 25, 2023

jbednar commented Dec 27, 2023

MarcoGorelli commented Dec 27, 2023 • edited Loading

stinodego left a comment

Choose a reason for hiding this comment

MarcoGorelli Dec 28, 2023

Choose a reason for hiding this comment

stinodego left a comment

Choose a reason for hiding this comment

stinodego Dec 31, 2023

Choose a reason for hiding this comment

MarcoGorelli Dec 31, 2023

Choose a reason for hiding this comment

stinodego Dec 31, 2023 • edited Loading

Choose a reason for hiding this comment

MarcoGorelli Dec 31, 2023

Choose a reason for hiding this comment

hoxbro commented Dec 31, 2023 • edited Loading

MarcoGorelli commented Dec 31, 2023

hoxbro commented Dec 31, 2023

stinodego left a comment

Choose a reason for hiding this comment

ritchie46 commented Dec 31, 2023

alonme commented Jan 2, 2024

MarcoGorelli commented Jan 2, 2024

MarcSkovMadsen commented Jan 2, 2024 • edited Loading

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

MarcoGorelli commented Dec 24, 2023 •

edited

Loading

MarcoGorelli commented Dec 27, 2023 •

edited

Loading

stinodego Dec 31, 2023 •

edited

Loading

hoxbro commented Dec 31, 2023 •

edited

Loading

MarcSkovMadsen commented Jan 2, 2024 •

edited

Loading