Skip to content

Deprecate plot_date() #18154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
timhoffm opened this issue Aug 2, 2020 · 6 comments · Fixed by #18346
Closed

Deprecate plot_date() #18154

timhoffm opened this issue Aug 2, 2020 · 6 comments · Fixed by #18346

Comments

@timhoffm
Copy link
Member

timhoffm commented Aug 2, 2020

This is a writeup / proposal to serve as a basis for discussion in the dev call on 03 Aug 2020.

plot_date() is essentially a thin wrapper around plot() but has different defaults for fmt. This causes confusion #17548, #18141.

Context

  • Use case: plot_date() dates back way before Matplotlib supported units. It's nowadays only necessary if you have plain numbers (in matplotlibs epoch). datetimes are directly handled in plot() via the unit system.
    While there may be users for plot_date(), i suppose it's rather a niche application.

  • Naming: The name is a bit misleading in that it suggests that you should use the function if you want to plot dates. This implicitly includes datetime / date which would be misleading. I'm suspecting that it may lead users with datetime/date objects on the wrong track. (plot_as_date() would have been more appropriate, but we're not gonna change that).

  • API design: plot_date() is an ad-hoc aggregation of plot() and axis_date()

      def plot_date(self, x, y, fmt='o', tz=None, xdate=True, ydate=False, **kwargs):
          if xdate:
              self.xaxis_date(tz)
          if ydate:
              self.yaxis_date(tz)
          return self.plot(x, y, fmt, **kwargs)
    

Recommendation

Given the above context, I propose to deprecate plot_date() in favor of ax.plot() and ax.x/yaxis.axis_date().

At the same time, deprecate ax.x/yaxis_date() in favor of ax.x/yaxis.axis_date(). This reduces the API footprint of Axes. IMHO we don't need to have wrappers x/yfoo() wrappers for every foo() function on the axis. While common ones like set_x/ylabel() have a justification, it's fine in general to use ax.xaxis.foo().

Alternatives:

  • Keep as is - I wouldn't do that as it's confusing users.
  • Change the default fmt to match plot() - Such a default change requires as much user action as the deprecation, but we keep all the unnecessary redundancy / imperfect naming.
  • Only soft-deprecate (i.e. add a note to discourage the use but keep the function). This would be an option if we're strongly concerned with not forcing users to change their plot_date() code.
@timhoffm
Copy link
Member Author

timhoffm commented Aug 2, 2020

I don't want to prefetch the dev call discussion to a lengthy thread here, so please don't comment unless you think it's absolutely crucial for preparation of the dev call.

@dstansby
Copy link
Member

dstansby commented Aug 7, 2020

Did this get discussed/are there any notes on the discussion from the dev call?

@timhoffm
Copy link
Member Author

timhoffm commented Aug 7, 2020

No, we did not come to this in the last call.

@jklymak
Copy link
Member

jklymak commented Aug 7, 2020

Is there scope for discussion of this off the call?

@timhoffm
Copy link
Member Author

timhoffm commented Aug 7, 2020

Is there scope for discussion of this off the call?

Do you mean you want to discuss this outside the call? If you have additional arguments, you can post them here. I just think reaching a decision is easiest when having a proposal and discussing that in the call.

BTW: I can't make it to next Monday's call. Either discuss this without me (my opinion is laid out above), or shift one week.

@jklymak
Copy link
Member

jklymak commented Aug 8, 2020

plot_date meets two, somewhat niche, needs that plot does not. 1) it allows setting the timezone, 2) it allows floats to be plotted using the date formatters and locators. Both needs are met by ax.xaxis_date, so I agree its a bit bonkers to have both wrappers, though I'd mildly prefer plot_date.

I am uncertain, however, about the ax.xaxis.foo paradigm. Most of https://matplotlib.org/api/axis_api.html seems to me more like low-level methods, mostly for internal use. i.e. do we think users should generally call ax.xaxis.set_label_text instead of ax.set_xlabel? Is the hope for ax.set_xticks to go away? I'm not even sure where the user would learn that an axes has an xaxis property, or how they would know to find the documentation for that class under matplotlib.axis.

So overall, I am mildly concerned about dropping plot_date, though I admit I never use it myself (because I use UTC times). At the very least we should think about how to make better sure that users can find the axis class if we are going to put non-advanced functionality on it.

normanius added a commit to hirsch-lab/mhealth that referenced this issue Apr 12, 2021
Related to matplotlib/matplotlib#18154 - inconsistent default of fmt argument
@QuLogic QuLogic added this to the v3.5.0 milestone May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants