CLN: Remove PeriodIndex.tz_convert, tz_localize #21935
Conversation
|
There's a linting problem in the test file. Other than that, assuming there's a consensus about removing these, LGTM. |
Codecov Report
@@ Coverage Diff @@
## master #21935 +/- ##
==========================================
+ Coverage 91.96% 91.96% +<.01%
==========================================
Files 166 166
Lines 50323 50329 +6
==========================================
+ Hits 46281 46287 +6
Misses 4042 4042
Continue to review full report at Codecov.
|
gfyoung
added
Error Reporting
doc/source/whatsnew/v0.24.0.txt
Outdated
| @@ -322,7 +322,7 @@ Removal of prior version deprecations/changes | |||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
|
|
|||
| - The ``LongPanel`` and ``WidePanel`` classes have been removed (:issue:`10892`) | |||
| - | |||
| - :meth:`PeriodIndex.tz_convert` and :meth:`PeriodIndex.tz_localize` have been removed (:issue:`21781`) | |||
There was a problem hiding this comment.
Should go in the Other API Changes section. We generally reserve this area for actual deprecations.
There was a problem hiding this comment.
Also I tried to document the changes made in api.rst, but removing the methods caused a bunch of errors in the build. Do you know what is causing that? Do I need to build the docs?
There was a problem hiding this comment.
Not sure ATM. If there's anything to remove in api.rst (or any of the .rst files), it would be reference to the methods that you just removed.
There was a problem hiding this comment.
can you move to Other API canges, otherwise lgtm.
doc/source/whatsnew/v0.24.0.txt
Outdated
| @@ -322,7 +322,7 @@ Removal of prior version deprecations/changes | |||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |||
|
|
|||
| - The ``LongPanel`` and ``WidePanel`` classes have been removed (:issue:`10892`) | |||
| - | |||
| - :meth:`PeriodIndex.tz_convert` and :meth:`PeriodIndex.tz_localize` have been removed (:issue:`21781`) | |||
There was a problem hiding this comment.
can you move to Other API canges, otherwise lgtm.
| @@ -779,10 +778,7 @@ def test_tz_convert_and_localize(self, fn): | |||
|
|
|||
| # TODO: l1 should be a PeriodIndex for testing | |||
| period_range('20140701', periods=1).tz_convert('UTC') | ||
| with pytest.raises(NotImplementedError): | ||
| period_range('20140701', periods=1).tz_localize('UTC') | ||
|
|
||
| # l1 = period_range('20140701', periods=5, freq='D') |
|
@jreback Everything look good? |
|
@jquinon : Everyone seems happy, so yes, indeed! Thanks! |
CLN: Removed tz_convert and tz_localize from PeriodIndex Closes pandas-devgh-21781
CLN: Removed tz_convert and tz_localize from PeriodIndex Closes pandas-devgh-21781
closes remove PeriodIndex.tz_convert, tz_localize? #21781
passes
git diff upstream/master -u -- "*.py" | flake8 --diffRemoved PeriodIndex.tz_convert and PeriodIndex.tz_localize from PeriodIndex.
Removed references to those methods in test_tz_convert_and_localize within test_timeseries
Removed methods from the PeriodIndex class doc and updated whatsnew
test_tz_convert_and_localize also contains a TODO referencing changing the test to use PeriodIndex for a timezone test, which if it is never gaining timezone info should probably be removed. I just wanted to check that this was a good idea before doing it.