Add dataarray scatter with 3d support

[Illviljan]

• 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]

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]

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

[Illviljan]

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:

[Illviljan]

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]

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]

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]

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