pydata / xarray

N-D labeled arrays and datasets in Python
https://xarray.dev
Apache License 2.0
3.65k stars 1.09k forks source link

hue_style is not documented in xarray.plot.scatter #9057

Closed dschwoerer closed 6 months ago

dschwoerer commented 6 months ago

What happened?

Looking at https://docs.xarray.dev/en/stable/generated/xarray.plot.scatter.html hue_style is mentioned, but not documented.

What did you expect to happen?

hue_style is documented. At least one of the following would be great: What are valid options? What do they do? Where can I find more info?

Minimal Complete Verifiable Example

No response

MVCE confirmation

Relevant log output

No response

Anything else we need to know?

https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.scatter.html#matplotlib.pyplot.scatter does not list hue_style, so think it is xarray specific.

Environment

I looked at the documentation using Firefox on Fedora Linux. This was observed on Fri May 31 08:59:55 AM UTC 2024
max-sixty commented 6 months ago

This is deprecated according to the changelog...

Illviljan commented 6 months ago

hue_style is being deprecated in favor of add_legend and add_colorbar:

https://github.com/pydata/xarray/blob/1f3bc7e96ec0b0c32250ec3f8e7e8a2d19d9a306/xarray/plot/dataarray_plot.py#L923-L934

scottyhq commented 6 months ago

@Illviljan @max-sixty, I'd guess most users are just seeing the docstring and not the deprecation warning since Python silences it by default (https://docs.python.org/2/library/warnings.html#default-warning-filters)

(To expose the warning requires extra config)

import warnings
warnings.filterwarnings("default") #or "module", or "always" 

ds = xr.tutorial.scatter_example_dataset(seed=42)
ds.isel(w=1, z=1).plot.scatter(x="x", y="y", hue="z", hue_style='discrete')

In any case, at what point should all occurrences of hue_style be removed - is there a precedent for elapsed time between declared deprecation and removal for something that seems relatively minor like this? If you think that's OK I can submit a pull request to remove it.

max-sixty commented 6 months ago

We don't have a strict rule on when to remove (I think?), but I would think we've had enough time given it's a small feature

(Thanks for the comment @Illviljan & @scottyhq — my reply was too brief; I should have mentioned removing it as a next step...)

max-sixty commented 6 months ago

deprecation warning since Python silences it by default (https://docs.python.org/2/library/warnings.html#default-warning-filters)

FYI running code from a notebook or script still raises the warning in Python 3

scottyhq commented 6 months ago

FYI running code from a notebook or script still raises the warning in Python 3

Huh, sorry for the outdated docs link! I am definitely using Python3 but am not seeing the hue_style deprecation warning. Others are visible... I poked around a bit but am really not sure why, maybe something with stacklevel=2 but just speculating. Can you confirm you see the same behavior?

import xarray as xr
ds = xr.tutorial.scatter_example_dataset(seed=42)
ds.drop([0], dim='x')   # DeprecationWarning
ds.isel(w=1, z=1).plot.scatter(x="x", y="y", hue="z", hue_style='discrete')   # Nothing
max-sixty commented 6 months ago

Yes you're right @scottyhq! I guess it is indeed the stack level, since it gets raised in xarray rather than in the caller:

warnings.filterwarnings('default')
ds.isel(w=1, z=1).plot.scatter(x="x", y="y", hue="z", hue_style='discrete')

# /Users/maximilian/workspace/xarray/xarray/plot/accessor.py:300: DeprecationWarning: hue_style is no longer used for plot1d plots and the argument will eventually be removed. Convert numbers to string for a discrete
 hue and use add_legend or add_colorbar to control which guide to display.
  return dataarray_plot.scatter(self._da, *args, **kwargs)

We have a decorator which finds the correct stack level which we should generally use.

For more major features we could switch, but I think this is small enough it's fine to remove (but no strong view)