Skip to content

DOC: Fix copy-pasted name in .day_name docstring #22544

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 1 commit into from
Sep 8, 2018
Merged

DOC: Fix copy-pasted name in .day_name docstring #22544

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 30 additions & 10 deletions pandas/core/arrays/datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,17 +793,27 @@ def month_name(self, locale=None):
"""
Return the month names of the DateTimeIndex with specified locale.

.. versionadded:: 0.23.0

Parameters
----------
locale : string, default None (English locale)
locale determining the language in which to return the month name
locale : str, optional
Locale determining the language in which to return the month name.
Default is English locale.
Copy link
Member

Choose a reason for hiding this comment

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

Where is pandas making the default English? Haven't looked in detail but assumed this would be system dependent

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 would expect that to be system independent (when programming, I'd rather have predictable results).

Code is implemented in get_date_name_field():

pandas/pandas/_libs/tslibs/fields.pyx

Lines 102 to 103 in 86e8f23

if locale is None:
names = np.array(DAYS_FULL, dtype=np.object_)

pandas/pandas/_libs/tslibs/fields.pyx

Lines 117 to 118 in 86e8f23

if locale is None:
names = np.array(MONTHS_FULL, dtype=np.object_)

DAYS_FULL and MONTHS_FULL are hard-coded in Pandas:

pandas/pandas/_libs/tslibs/ccalendar.pyx

Lines 39 to 48 in 86e8f23

MONTHS_FULL = ['', 'January', 'February', 'March', 'April', 'May', 'June',
'July', 'August', 'September', 'October', 'November',
'December']
MONTH_NUMBERS = {name: num for num, name in enumerate(MONTHS)}
MONTH_ALIASES = {(num + 1): name for num, name in enumerate(MONTHS)}
MONTH_TO_CAL_NUM = {name: num + 1 for num, name in enumerate(MONTHS)}
DAYS = ['MON', 'TUE', 'WED', 'THU', 'FRI', 'SAT', 'SUN']
DAYS_FULL = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday',
'Saturday', 'Sunday']

Copy link
Member

Choose a reason for hiding this comment

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

The idea was for day_name to replace the depreciated weekday_name which defaulted to hardcoded english names (without localization support).

Copy link
Contributor

Choose a reason for hiding this comment

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

agree this should say the default is based on the locale

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 As you can see in the code snippets above, it is not based on the locale. The default is always English.

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 I do not think the code I am running is relevant here. Unless you refer to a trivial example like the one I added in the docstring (but why would you need that?):

idx = pd.DatetimeIndex(start='2018-09-02', freq='D', periods=3)
idx.day_name()

Can you explain why you still say the default when locale=None is based on the system's locale? I checked the tests, the code and the behavior and everything seems to prove otherwise: the default when locale=None is English, and hence the proposed docstring. Did you check anything?

Copy link
Contributor Author

@Peque Peque Sep 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@jreback If you tell me exactly what changes to the current docstring you have on your mind (it seems other reviewers are okay with it), I may better understand what you are saying. ❤️

Copy link
Member

Choose a reason for hiding this comment

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

@Peque @jreback The default locale is English as day_name is meant to replace weekday_name which defaulted to english. So the docstring here is correct.

@Peque feel free to open up a new issue suggesting that day_name and month_name should default to system locale. This would be an API change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mroeschke Yeah, but I do prefer the current behavior, so I would rather not open that issue (and note I am not a native English speaker nor live in an English country).

When programming, I prefer everything in English by default. 😜

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha @Peque. I hold a similar opinion for API transition sake.

Therefore, I think there's no action with this docstring here as far as I'm concerned.


Returns
-------
month_names : Index
Index of month names
Index
Index of month names.

.. versionadded:: 0.23.0
Examples
--------
>>> idx = pd.DatetimeIndex(start='2018-01', freq='M', periods=3)
>>> idx
DatetimeIndex(['2018-01-31', '2018-02-28', '2018-03-31'],
dtype='datetime64[ns]', freq='M')
>>> idx.month_name()
Index(['January', 'February', 'March'], dtype='object')
"""
if self.tz is not None and self.tz is not utc:
values = self._local_timestamps()
Expand All @@ -819,17 +829,27 @@ def day_name(self, locale=None):
"""
Return the day names of the DateTimeIndex with specified locale.

.. versionadded:: 0.23.0

Parameters
----------
locale : string, default None (English locale)
locale determining the language in which to return the day name
locale : str, optional
Locale determining the language in which to return the day name.
Default is English locale.

Returns
-------
month_names : Index
Index of day names
Index
Index of day names.

.. versionadded:: 0.23.0
Examples
--------
>>> idx = pd.DatetimeIndex(start='2018-01-01', freq='D', periods=3)
>>> idx
DatetimeIndex(['2018-01-01', '2018-01-02', '2018-01-03'],
dtype='datetime64[ns]', freq='D')
>>> idx.day_name()
Index(['Monday', 'Tuesday', 'Wednesday'], dtype='object')
"""
if self.tz is not None and self.tz is not utc:
values = self._local_timestamps()
Expand Down