Skip to content
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

MNT: deprecate epoch2num/num2epoch #19483

Merged
merged 1 commit into from
May 11, 2021

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Feb 8, 2021

PR Summary

epoch2num and num2epoch convert to and from matplotlib times from unix times.

We do not use these functions anywhere internally, and their functionality is provided by datetime or datetime64 modules. There is really no reason for users to be converting directly from unix times to Matplotlib times.

This was deprecated before when we made the epoch change, but that broke pandas who were using this. They no longer are using this, so I think it is safe to deprecate for 3.5.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@jklymak jklymak added this to the v3.5.0 milestone Feb 8, 2021
@jklymak
Copy link
Member Author

jklymak commented Feb 9, 2021

I'm conflicted about julian2num. It is obscure enough that many folks might be counting on it.

@jklymak
Copy link
Member Author

jklymak commented May 2, 2021

I'll ping for this simple one...

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the first relase of pandas that does not depend on this? I think we need to give it "some time" to settle.

I'm fine with deprecation the julian functions. I consider them a is too specifc and not part of the core functionality that we want to provide. The few potential users of this should be able to copy the code or get the conversion from somewhere else.

@@ -1081,18 +1081,6 @@ def test_change_interval_multiples():
assert ax.get_xticklabels()[1].get_text() == 'Feb 01 2020'


def test_epoch2num():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we leave tests for deprecated features in place and run them with _api.suppress_matplotlib_deprecation_warning():.

@jklymak
Copy link
Member Author

jklymak commented May 3, 2021

It was merged almost a year ago: pandas-dev/pandas#35393 milestoned 1.1, released Jul 28, 2020. They are currently on 1.2.4. I would guess that if someone updates Matplotlib they would also update pandas? Worse that happens is someone gets a deprecation warning for a while if they use a new Matplotlib and an old pandas.

@timhoffm
Copy link
Member

timhoffm commented May 3, 2021

That's ok then.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modulo one comment

lib/matplotlib/tests/test_dates.py Outdated Show resolved Hide resolved
@timhoffm timhoffm merged commit a9c5224 into matplotlib:master May 11, 2021
@jklymak jklymak deleted the mnt-deprecate-epoch2num branch May 11, 2021 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants