Add dataarray scatter with 3d support #4909

Illviljan · 2021-02-14T11:15:50Z

Closes #xxxx
Tests added
Passes pre-commit run --all-files
User visible changes (including notable bug fixes) are documented in whats-new.rst
New functions/methods are listed in api.rst

Add scatter plots for dataarrays. Then later remove the dataset version and replace it with a thin wrapper in a similar fashion to #4820.

New stuff:

Legend now displays colors and sizes in a similar fashion to Seaborn.
New parameter z which allows 3d scatter plots.

Example:
Using ds = xr.tutorial.scatter_example_dataset():

pep8speaks · 2021-02-14T11:15:53Z

Hello @Illviljan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-06-30 16:24:25 UTC

dcherian · 2021-02-17T00:11:30Z

that size legend is nice! I wonder if we should put both legends outside the axis using figlegend?

Illviljan · 2021-02-21T13:49:28Z

figlegend doesn't work that great within subplots though right? When I tried subplots the legends were just added on top of each other. I think ax.legend is a better, but it seems rather difficult to fit it outside. I've tried a few variants with bbox_to_anchor but it quickly gets non robust when deviating from the defaults.

I've been trying to merge the legends together though . So then the legend will attempt to find the best places automatically so that helps a little bit:

Remove the scatter for loop when using discrete values.

Illviljan · 2021-05-28T05:53:32Z

If it seems safer we could make this non-public, da.plot._scatter?
This is not being tested as properly as ds.plot.scatter at the moment so I might've missed some edge cases, but once we use the same code path for both of them this should be more reliable.

TomNicholas

@Illviljan this is an excellent PR!

It would be nice to factor out some of the helper functions into plot.utils or elsewhere, and especially to use the @_plot2d decorator, but there is not need to squeeze that into this PR.

If you're worried about this not being fully robust until other plotting methods are condensed into one code path then I would just suggest not making it callable as a plot method yet (or renaming to _scatter as you suggested). But I also think it's fine to release as is.

Illviljan · 2021-06-30T06:05:54Z

It would be nice to factor out some of the helper functions into plot.utils or elsewhere, and especially to use the @_plot2d decorator, but there is not need to squeeze that into this PR.

Thanks, @TomNicholas. My original plan and attempts was to use _plot2d. But because the return type originally was so different to the other plot functions (list of pathcollections vs pathcollections) it was impossible to use it without pretty much if-casing the entire _plot2d. I also feel _plot2d makes it pretty hard to read and understand the code.

I think it might be doable now that the function only returns pathcollection. I'll take a stab at that again in a different PR.

If you're worried about this not being fully robust until other plotting methods are condensed into one code path then I would just suggest not making it callable as a plot method yet (or renaming to _scatter as you suggested). But I also think it's fine to release as is.

Ok, I'll rename it to a private name. But I think it's good that it's still callable, so that others can easily try it out without any promises that the API will stay the same. I was also considering changing/removing some variable names related to legend compared to the dataset version, so it's good to let that it be private until that's been figured out.

github-actions · 2021-06-30T15:49:23Z

Unit Test Results

        6 files ±0         6 suites ±0 52m 40s ⏱️ ±0s
16 178 tests ±0 14 447 ✔️ ±0 1 730 💤 ±0 1 ❌ ±0
90 264 runs ±0 82 111 ✔️ ±0 8 147 💤 ±0 6 ❌ ±0

For more details on these failures, see this check.

Results for commit 4cde773. ± Comparison against base commit 4cde773.

♻️ This comment has been updated with latest results.

dcherian · 2021-07-17T21:07:26Z

Thanks @Illviljan We can keep iterating on this per Tom's comments.

* main: (31 commits) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) pre-commit: autoupdate hook versions (pydata#5617) Add dataarray scatter with 3d support (pydata#4909) ...

* upstream/main: (31 commits) Refactor index vs. coordinate variable(s) (pydata#5636) pre-commit: autoupdate hook versions (pydata#5685) Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670) Speed up _mapping_repr (pydata#5661) update the link to `scipy`'s intersphinx file (pydata#5665) Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663) pre-commit: autoupdate hook versions (pydata#5660) fix the binder environment (pydata#5650) Update api.rst (pydata#5639) Kwargs to rasterio open (pydata#5609) Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633) new blank whats-new for v0.19.1 v0.19.0 release notes (pydata#5632) remove deprecations scheduled for 0.19 (pydata#5630) Make typing-extensions optional (pydata#5624) Plots get labels from pint arrays (pydata#5561) Add to_numpy() and as_numpy() methods (pydata#5568) pin fsspec (pydata#5627) pre-commit: autoupdate hook versions (pydata#5617) Add dataarray scatter with 3d support (pydata#4909) ...

add dataarray scatter

8d71cc5

Illviljan added 2 commits February 14, 2021 12:16

Update plot.py

2dff1ce

Update plot.py

6837a38

Illviljan marked this pull request as draft February 14, 2021 11:32

Illviljan added 2 commits February 14, 2021 12:35

Update plot.py

1364e63

copy doc

66850f2

Illviljan added 7 commits February 19, 2021 22:00

Update plot.py

487770a

Merge branch 'master' into Illviljan-dataarray_scatter

5ee8669

merge hue and size legends

88ce1c8

Update plot.py

5215c7a

add test

bada616

Update plot.py

ccc1b35

add 3d support

fccc1d7

Illviljan added 14 commits February 21, 2021 16:31

fix labels

059ffc0

Update plot.py

f281634

remove ax.scatter loop

99eb5c1

Remove the scatter for loop when using discrete values.

Update plot.py

8fd4765

Update plot.py

6eceabe

use label mapper only when necessary

1a5c2da

Update plot.py

34d1e74

Update plot.py

fb0d7ca

func should return np.arrays

a65a6d9

better defaults in legend_elements

b510bdc

use pd.unique to retain order

6814c6b

update legend_:elements

7a9e4d5

Merge branch 'master' into Illviljan-dataarray_scatter

d301f6a

Update plot.py

e91aaba

suggestions from review

1626174

Illviljan mentioned this pull request Apr 20, 2021

Surface plots #5101

Merged

5 tasks

Merge branch 'master' into Illviljan-dataarray_scatter

3fada9f

dcherian requested a review from TomNicholas May 13, 2021 17:44

Illviljan added 4 commits May 18, 2021 20:14

Merge branch 'master' into Illviljan-dataarray_scatter

17bb3b5

view_init(30, 30)

f8de23a

Add support for new stuff in matplotlib master

fda0310

Update plot.py

6eb190b

TomNicholas approved these changes Jun 23, 2021

View reviewed changes

Illviljan added 3 commits June 24, 2021 18:24

Merge branch 'main' into Illviljan-dataarray_scatter

56351f2

fix merge errors

63b064f

Update test_plot.py

530c580

Illviljan closed this Jun 24, 2021

Illviljan reopened this Jun 24, 2021

privatize scatter plot for now.

85e4ecd

Illviljan closed this Jun 30, 2021

Illviljan reopened this Jun 30, 2021

Illviljan closed this Jun 30, 2021

Illviljan reopened this Jun 30, 2021

TomNicholas added the plan to merge Final call for comments label Jun 30, 2021

TomNicholas mentioned this pull request Jul 8, 2021

Release v0.19? #5588

Closed

8 tasks

dcherian merged commit 4cde773 into pydata:main Jul 17, 2021

Illviljan mentioned this pull request Jun 14, 2023

plot.scatter(hue_style="discrete") does nothing #7907

Closed

4 tasks

Illviljan deleted the Illviljan-dataarray_scatter branch March 13, 2024 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dataarray scatter with 3d support #4909

Add dataarray scatter with 3d support #4909

Illviljan commented Feb 14, 2021 •

edited

Loading

pep8speaks commented Feb 14, 2021 •

edited

Loading

dcherian commented Feb 17, 2021

Illviljan commented Feb 21, 2021

Illviljan commented May 28, 2021

TomNicholas left a comment

Illviljan commented Jun 30, 2021

github-actions bot commented Jun 30, 2021 •

edited

Loading

dcherian commented Jul 17, 2021

Add dataarray scatter with 3d support #4909

Add dataarray scatter with 3d support #4909

Conversation

Illviljan commented Feb 14, 2021 • edited Loading

pep8speaks commented Feb 14, 2021 • edited Loading

Comment last updated at 2021-06-30 16:24:25 UTC

dcherian commented Feb 17, 2021

Illviljan commented Feb 21, 2021

Illviljan commented May 28, 2021

TomNicholas left a comment

Choose a reason for hiding this comment

Illviljan commented Jun 30, 2021

github-actions bot commented Jun 30, 2021 • edited Loading

Unit Test Results

dcherian commented Jul 17, 2021

Illviljan commented Feb 14, 2021 •

edited

Loading

pep8speaks commented Feb 14, 2021 •

edited

Loading

github-actions bot commented Jun 30, 2021 •

edited

Loading