DEPR: Deprecate pandas.datetime by ryankarlos · Pull Request #30489 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation19 Commits13 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- closes DEPR: Remove pandas.datetime #30610
- tests added / passed
- passes
black pandas - passes
git diff upstream/master -u -- "*.py" | flake8 --diff - whatsnew entry
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
Depr/remove pandas.datetime Deprecate pandas.datetime
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
Deprecate pandas.datetime DEPR: Deprecate pandas.datetime
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 ?
@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.
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!
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.
| try: |
|---|
| return getattr(self.np, item) |
| except AttributeError: |
| raise AttributeError(f"module numpy has no attribute {item}") |
| np = __numpy() |
| class __Datetime: |
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?
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
| def __getattr__(self, item): |
| self.warnings.warn( |
| "The pandas.datetime module is deprecated " |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as before
12 tasks