Skip to content

DOC: Updated docstring DatetimeIndex.tz_localize #20050

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

Merged
merged 20 commits into from
Mar 14, 2018
Merged

DOC: Updated docstring DatetimeIndex.tz_localize #20050

Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9bd697d
Updated DocString DatetimeIndex.tz_localize
Mar 8, 2018
ca813e6
Changes according to instruction
Mar 8, 2018
a5a6a09
updated Docstring
Mar 8, 2018
44b86cd
indentation issue
Mar 8, 2018
1fe3b34
update Indentation
Mar 8, 2018
b757763
Final Updated docString
IHackPy Mar 9, 2018
fba3685
Final Updated Docstring
IHackPy Mar 9, 2018
5e7b205
Final Updated Docstring
IHackPy Mar 9, 2018
aca527e
added infer_dst
IHackPy Mar 9, 2018
8954ea8
Add Reverse Example & update summary
IHackPy Mar 10, 2018
67e2597
Update df to dti
IHackPy Mar 10, 2018
a791db1
Update final required change
IHackPy Mar 10, 2018
99a846c
change example ipython mode to terminal mode
IHackPy Mar 10, 2018
3fa0a2e
Add comment on code
IHackPy Mar 11, 2018
c81bd02
Changes according guidance
IHackPy Mar 11, 2018
bdc839b
changes according guidance
IHackPy Mar 11, 2018
fa75bf5
update with return and example section
IHackPy Mar 11, 2018
85f6dbe
Example Section
IHackPy Mar 11, 2018
6205b61
Update changes
IHackPy Mar 11, 2018
95fcee2
Update datetimes.py
jorisvandenbossche Mar 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions pandas/core/indexes/datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1937,16 +1937,22 @@ def tz_convert(self, tz):
mapping={True: 'infer', False: 'raise'})
def tz_localize(self, tz, ambiguous='raise', errors='raise'):
"""
Localize tz-naive DatetimeIndex to given time zone (using
pytz/dateutil), or remove timezone from tz-aware DatetimeIndex
Localize tz-naive DatetimeIndex to tz-aware DatetimeIndex.

This method takes a naive DatetimeIndex object and make this
time zone aware. It does not move the time to another
timezone.
Copy link
Member

Choose a reason for hiding this comment

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

I found the interprets word a bit unclear. I think something like "Make a naive DatetimeIndex time zone aware, by attaching a time zone. It does not change the values."

Surely you can write it in a better way, but hopefully you see what I mean.

Copy link
Contributor Author

@IHackPy IHackPy Mar 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@datapythonista hows this otherwise I'm going to co[py paste same you suggest above :)

"This method takes a naive DatetimeIndex object and make this time zone aware. It does not move the time to another timezone.
Time zone localization helps to switch b/w time zone aware and time zone unaware objects."

Time zone localization helps to switch b/w time zone aware and time
zone unaware objects.

Parameters
----------
tz : string, pytz.timezone, dateutil.tz.tzfile or None
Time zone for time. Corresponding timestamps would be converted to
time zone of the TimeSeries.
None will remove timezone holding local time.
ambiguous : 'infer', bool-ndarray, 'NaT', default 'raise'
ambiguous : str {'infer', 'NaT', 'raise'} or bool array, \
default 'raise'
- 'infer' will attempt to infer fall dst-transition hours based on
order
- bool-ndarray where True signifies a DST time, False signifies a
Expand All @@ -1955,7 +1961,7 @@ def tz_localize(self, tz, ambiguous='raise', errors='raise'):
- 'NaT' will return NaT where there are ambiguous times
- 'raise' will raise an AmbiguousTimeError if there are ambiguous
times
errors : 'raise', 'coerce', default 'raise'
errors : {'raise', 'coerce'}, default 'raise'
- 'raise' will raise a NonExistentTimeError if a timestamp is not
valid in the specified timezone (e.g. due to a transition from
or to DST time)
Expand All @@ -1968,14 +1974,47 @@ def tz_localize(self, tz, ambiguous='raise', errors='raise'):
.. deprecated:: 0.15.0
Attempt to infer fall dst-transition hours based on order
Copy link
Member

Choose a reason for hiding this comment

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

You can leave this one (although it will give an error in the validation script, but you can ignore that)

Copy link
Contributor Author

@IHackPy IHackPy Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I will add this again.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorisvandenbossche why adding this back? looks leftover

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback @jorisvandenbossche I add this again or not ???????

Copy link
Member

Choose a reason for hiding this comment

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

@jreback no, the deprecation is still there (the fact that the deprecation is still there is certainly a left-over, but let's remove that (both the actual deprecation and the docs) in a separate PR)

@himanshuawasthi95 Yes, please add back



Returns
-------
localized : DatetimeIndex
DatetimeIndex

Examples
--------
In the example below, We put the date range from 01 March 2018
to 08 March 2018 & localize this to US/Eastern Time zone & again
we perform reverse operation where we remove tize zone & make it
Copy link
Member

Choose a reason for hiding this comment

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

Can you just leave the type in Returns (so, get rid of the "localized" name). Also, a short explanation on what is returned would be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@datapythonista
I'm understand that I have to remove localized from return section & write like this :
Returns

DatetimeIndex

or I should write some explanation on returns section like this:
Returns

Some Explanation
I'm confused about it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and parameter ambiguous : {'infer', 'NaT', default 'raise'}, bool-ndarray . write like this ???

Copy link
Member

Choose a reason for hiding this comment

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

For return, first line as you say, then another line with explanation. You can check the first example here:
https://python-sprints.github.io/pandas/guide/pandas_docstring.html#section-4-returns-or-yields.

For parameter, I didn't see a case exactly like this before, but for the convention I'd say something like:

{'raise', 'infer', 'NaT'} or bool array-like, default 'raise'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there any change with summary & with example section

tz-naive

>>> dti = pd.date_range('2018-03-01', '2018-03-03')

>>> dti
DatetimeIndex(['2018-03-01', '2018-03-02', '2018-03-03'],
Copy link
Member

Choose a reason for hiding this comment

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

Returns should contain a description. See the example in: https://python-sprints.github.io/pandas/guide/pandas_docstring.html#section-4-returns-or-yields

dtype='datetime64[ns]', freq='D')

localize DatetimeIndex in US/Eastern time zone.
Copy link
Member

Choose a reason for hiding this comment

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

The explanations of the examples need improvements. The first paragraph is outdated (end date is not the 8th). And I don't think you need to say that, it's clear from the code. So I would just get rid of it.

In this case, it should start with a capital L. May be you can add here what you were explaining before, that dti is naive (and also what you're already saying, that you're localizing to US/Eastern). Personally I'd rename dti to tz_aware, so it's more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are saying this :

In [2]: tz_aware = pd.date_range('2018-03-01', '2018-03-03')

In [3]: tz_aware
Out[3]: DatetimeIndex(['2018-03-01', '2018-03-02', '2018-03-03'], dtype='datetime64[ns]', freq='D')

In [4]: tz_aware.tz_localize(tz='US/Eastern')
Out[4]:
DatetimeIndex(['2018-03-01 00:00:00-05:00', '2018-03-02 00:00:00-05:00',
'2018-03-03 00:00:00-05:00'],
dtype='datetime64[ns, US/Eastern]', freq='D')

In [5]: tz_aware.tz_localize(None)
Out[5]: DatetimeIndex(['2018-03-01', '2018-03-02', '2018-03-03'], dtype='datetime64[ns]', freq='D')

Copy link
Member

Choose a reason for hiding this comment

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

Almost. But in In [2] that's not a tz_aware index, it's a naive one. And in In [4] you do want to save the output, as that's the aware one that you'll one to convert to naive in In [5].

Make sure that the example makes sense for you too. Reviewers can also be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but thats why we added comment before where we analyse aware and naive

Copy link
Member

Choose a reason for hiding this comment

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

I agree, but I think the variable names can still help by being descriptive, even if you explain what you're doing. Feel free to keep dti if you prefer it over tz_naive. But for me it's more clear to see what the example does, if the variables are more descriptive. All objects in the example could be dti, and it doesn't help much IMO, but naming one tz_naive and the other tz_aware, makes it clear what every object used has.

Sorry if I'm not being able to explain what I mean clearer enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope this commit define exactly what you want in example section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for disappointing you :(


>>> tz_aware = dti.tz_localize(tz='US/Eastern')

>>> tz_aware
DatetimeIndex(['2018-03-01 00:00:00-05:00',
'2018-03-02 00:00:00-05:00', '2018-03-03 00:00:00-05:00'],
dtype='datetime64[ns, US/Eastern]', freq='D')

localize aware time zone into naive time zone.
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.


>>> tz_naive = tz_aware.tz_localize(None)

>>> tz_naive
Copy link
Member

Choose a reason for hiding this comment

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

As you're not doing anything with tz_naive more than displaying it, I wouldn't save it to a variable first, I'd show the output direclty.

DatetimeIndex(['2018-03-01', '2018-03-02', '2018-03-03'],
dtype='datetime64[ns]', freq='D')


Raises
------
TypeError
If the DatetimeIndex is tz-aware and tz is not None.

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 don't leave this line here, the validation script probably complains.

"""
if self.tz is not None:
if tz is None:
Expand Down