ENH: New level argument for DataFrame.tz_localize and DataFrame.tz_convert (GH7846)

[mtrbean]

Closes #7846

New level argument for DataFrame.tz_localize() and DataFrame.tz_convert(), needed for a DataFrame with MultiIndex:

tz_convert(self, tz, axis=0, level=None, copy=True)
tz_localize(self, tz, axis=0, level=None, copy=True, infer_dst=False)

@jreback Not sure if test_generic.py is the right place to add the tests.

[jreback]

looks good, can you add a test with l1 being a DatetimeIndex you are trying to localize/convert and l0 is something else (say an int index). obviously should only work with l1. Also add tests using a PeriodIndex (I think should work similary to DatetimeIndex). You can combine both of these tests into 1 and just loop over: tz_convert/tz_localize, and PeriodIndex/Datetime if you want (to avoid duplicating code)

[mtrbean]

I have added tests you suggested and condensed the testing code but unfortunately I just realized that PeriodIndex does not have full tz support. So only datetimeindex is tested :(

[jreback]

hmm, can you show an example of what fails? it should work

[mtrbean]

I didn't spend too much time on it, but these are things that I noticed on the master branch:

• period_range doesn't accept a tz param like date_range (which is easy to fix)
• Then I tried to construct a PeriodIndex directly with timezone. However it doesn't have tz_convert nor tz_localize methods(!)

Maybe make tz_convert and tz_localize part of the DatetimeIndexOpsMixin?, but anyway I feel that it warrants opening a separate GH issue.

[jreback]

hmm, yes, that looks likely (prob just never done).

@sinhrks do we have an issue about those?

[sinhrks]

Yep (#2106).

@mtrbean Because of internal representations are different, current DTI logic can't work for PeriodIndex. Or you mean to add dummy func to raise an error?

[jreback]

@mtrbean ok, in that vein, let's just not do PeriodIndex! (which I think you did already)

do you have a test for raise if only have a PeriodIndex and you try tz_localize/tz_convert (on a multi-index)? (I think this test already exists if the index is a PeriodIndex, e.g. its not a DatetimeIndex).

[mtrbean]

@jreback sorry not sure exactly what you meant. Should I add dummy PeriodIndex.tz_convert and tz_localize that raise NotImplementedError?

For the tests shall we do something along the line of the following?

try:
    PeriodIndex().tz_convert()
except NotImplementedError:
    nose.SkipTest('Not tz support for PeriodIndex yet')

[mtrbean]

@jreback I have made some changes to the test so that it does not test PeriodIndex if not implemented. Let me know if you like it or not.

[jreback]

this is going to get lost. take this back out, and just do DatetimeIndex (put a note on that issue though)

[jreback]

actually I take that back, nvm.

[jreback]

why do you want the else here? when PeriodIndex is eventually changed this should break.

[jreback]

ok, looks good, pls squash down to a single commit.

If you want (could be here or another PR). If you want to make a factory function to generate tz_localize/tz_convert in core/generic (see _make_stat_function for how you would do this). To avoid some duplicate code. Its not a big deal (becuase its only 2 functions). lmk if you want to do this (or if its not worth it0

[mtrbean]

Squashed into a single commit.

I don't think it is worth doing it at this point, as the function signature of tz_localize and tz_convert are slightly different (tz_localize has infer_dst but the other one does not). Let me think about it more and will create another PR if I can think of something that makes more sense.

[jreback]

ok, ping when green

[mtrbean]

Tests passed!

[jreback]

thanks!

nice fix!