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.

Sign up for GitHub

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

Jump to bottom

DEPR: Deprecate pandas.datetime #30489

Merged
merged 13 commits into from Jan 2, 2020
Merged

DEPR: Deprecate pandas.datetime #30489

merged 13 commits into from Jan 2, 2020

Conversation

ryankarlos
Copy link
Contributor

@ryankarlos ryankarlos commented Dec 26, 2019 •
edited

@pep8speaks
Copy link

pep8speaks commented Dec 26, 2019 •
edited

Hello @ryankarlos! 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 2020-01-02 02:08:34 UTC

@ryankarlos ryankarlos changed the title Depr/remove pandas.datetime Deprecate pandas.datetime Dec 26, 2019
alimcmaster1
alimcmaster1 reviewed Dec 26, 2019
View reviewed changes
pandas/__init__.py Outdated Show resolved Hide resolved
alimcmaster1
alimcmaster1 reviewed Dec 26, 2019
View reviewed changes
Copy link
Member

@alimcmaster1 alimcmaster1 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR - left a few minor comments

@alimcmaster1 alimcmaster1 added the Deprecate Functionality to remove in pandas label Dec 26, 2019
@ryankarlos ryankarlos changed the title Deprecate pandas.datetime DEPR: Deprecate pandas.datetime Dec 26, 2019
@ryankarlos
Copy link
Contributor Author

ryankarlos commented Dec 27, 2019 •
edited

having some issues with adding this Datetime class for py36 - was trying to keep it consistent with the way np is handled - i get an error when trying to call pd.datetime in the test - can do pd.datetime.datetime although then there is the problem of warning not been generated due to _getattr not being accessedd correctly.

Also, original issue i had linked this PR with is now closed - do i need to open a new one for datetime ?

@lithomas1
Copy link
Member

lithomas1 commented Dec 29, 2019 •
edited

@ryankarlos
The intention of the test_depr() function is to test that the getattr() function of the wrapper class(e.g. __numpy() or __datetime() in your case) raises a warning.

@alimcmaster1
Copy link
Member

alimcmaster1 commented Jan 1, 2020 •
edited

Also, original issue i had linked this PR with is now closed - do i need to open a new one for datetime ?

Yes please - and link the original issue. Also now that #30386 has been merged the implementation could be simplified - similar to what we do for SparseSeries/SparseDataFrame.

Also can you merge master + take a look at test failures.

Thanks!

datapythonista
datapythonista reviewed Jan 1, 2020
View reviewed changes
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Nice change, just a minor thing.

You'll have to fix the conflicts too.

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
pandas/__init__.py Outdated Show resolved Hide resolved
pandas/__init__.py
try:
return getattr(self.np, item)
except AttributeError:
raise AttributeError(f"module numpy has no attribute {item}")

np = __numpy()

class __Datetime:
Copy link
Member

Choose a reason for hiding this comment

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

Why two underscores instead of one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just to make it consistent with how __numpy() was being used - same for get__attr as thats how its used in pandas/tests/api/test_api.py::TestPDApi::test_depr
but i can revert if still needed

pandas/__init__.py Outdated

def __getattr__(self, item):
self.warnings.warn(
"The pandas.datetime module is deprecated "
Copy link
Member

Choose a reason for hiding this comment

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

same as before

datapythonista
datapythonista reviewed Jan 2, 2020
View reviewed changes
doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
pandas/__init__.py Outdated Show resolved Hide resolved
@ryankarlos
Copy link
Contributor Author

@alimcmaster1 @datapythonista all green now - think ive addressed the required changes.

@jorisvandenbossche jorisvandenbossche mentioned this pull request Jan 2, 2020
Closed
@jreback jreback added this to the 1.0 milestone Jan 2, 2020
@jreback jreback merged commit 0913ed0 into pandas-dev:master Jan 2, 2020
@jreback
Copy link
Contributor

jreback commented Jan 2, 2020

very nice @ryankarlos thanks!

@ryankarlos ryankarlos deleted the DEPR/remove-pandas.datetime branch January 2, 2020 04:24
@mroeschke mroeschke mentioned this pull request Oct 20, 2022
Merged
@tjni tjni mentioned this pull request Aug 30, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datapythonista datapythonista datapythonista left review comments

@alimcmaster1 alimcmaster1 alimcmaster1 left review comments

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

DEPR: Remove pandas.datetime
6 participants
@ryankarlos @pep8speaks @lithomas1 @alimcmaster1 @jreback @datapythonista