Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: support .strftime for datetimelikes (closes #10086) #10110

Merged
merged 1 commit into from
Aug 3, 2015

Conversation

mortada
Copy link
Contributor

@mortada mortada commented May 12, 2015

closes #10086

@jreback jreback added Enhancement Output-Formatting __repr__ of pandas objects, to_string labels May 12, 2015
@jreback jreback added this to the 0.17.0 milestone May 12, 2015
@jreback jreback added Datetime Datetime data dtype Strings String extension data type and string data labels May 12, 2015
@jreback
Copy link
Contributor

jreback commented May 12, 2015

this works for DatetimeIndex, something broken for PeriodIndex. And should not work for TimedeltaIndex (as I don't think the strftime codes are meaningful). Further want to actually define .strftime on the index itself as well (maybe def strftime(self, format):).

needs tests for the same

@mortada
Copy link
Contributor Author

mortada commented May 13, 2015

that makes sense

I've added to both DatetimeIndex and PeriodIndex. Also added a few tests.

@jreback
Copy link
Contributor

jreback commented May 13, 2015

pls add strftime to API.rst
also I think a mini section in time series.rst would be ok too

@@ -694,6 +694,21 @@ def astype(self, dtype):
else: # pragma: no cover
raise ValueError('Cannot cast DatetimeIndex to dtype %s' % dtype)

def strftime(self, date_format):
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, not to make this more complicated, but maybe move to tseries/base/DatelikeOps (new mixin), that both DatetimeIndex/PeriodIndex, but NOT TimedeltaIndex inherit from.

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 thought about something like that too, good idea for code reuse! Will update shortly

@mortada
Copy link
Contributor Author

mortada commented May 14, 2015

@jreback it's updated

@mortada
Copy link
Contributor Author

mortada commented Jun 1, 2015

@jreback I added a mini section to basics.rst instead, but otherwise this is ready for review

@@ -1164,12 +1164,28 @@ You can also chain these types of operations:

s.dt.tz_localize('UTC').dt.tz_convert('US/Eastern')

You can also format the datetime values with ``Series.dt.strftime``:

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe be explict and say that this is a string

@jreback
Copy link
Contributor

jreback commented Jun 26, 2015

can you update for comments. also related to #10442. I want this method to be the 'primary' date-string formatting (though .astype(str) will be supported by calling what this calls, namely .format), but this should be the goto method

copy your example that you added to basics.rst and put in 0.17.0 (in a separate section, maybe datetime-string formatting or somesuch)

@mortada
Copy link
Contributor Author

mortada commented Jun 27, 2015

@jreback updated according to your comments except for one thing: I couldn't actually find an invalid formatting string that would cause strftime to raise an exception

@@ -1238,12 +1238,28 @@ You can also chain these types of operations:

s.dt.tz_localize('UTC').dt.tz_convert('US/Eastern')

You can also format datetime values as strings with ``Series.dt.strftime``:

.. ipython:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

can u add a reference to the standard library docs for the strftime format here

@@ -32,6 +32,23 @@ Other enhancements
^^^^^^^^^^^^^^^^^^

- ``.as_blocks`` will now take a ``copy`` optional argument to return a copy of the data, default is to copy (no change in behavior from prior versions), (:issue:`9607`)
- Support ``.strftime`` for datetime-likes (:issue:`10110`)

For example:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this all needs to be indented 2 spaces (to form a sub-block)

@jorisvandenbossche
Copy link
Member

We also have a tslib.format_array_from_datetime function. Can you check if this is faster? (also uses strftime, but the loop is cythonized)

@mortada
Copy link
Contributor Author

mortada commented Jul 30, 2015

@jorisvandenbossche sure I'll check

imask = ~mask
values[imask] = np.array([u('%s') % dt for dt in values[imask]])

formatter = lambda dt: dt.strftime(date_format) if date_format else u('%s') % dt
Copy link
Contributor

Choose a reason for hiding this comment

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

put the if on the outside (and lambdas on the inside), then check is only done once

@jreback
Copy link
Contributor

jreback commented Jul 30, 2015

@jorisvandenbossche this calls .format(date_format=...) which ultimately calls the cython tslib.format_array_from_datetime, so this is just syntatic sugar.

@mortada
Copy link
Contributor Author

mortada commented Jul 30, 2015

@jreback reordered the lambdas and the if/else as you suggested, also added more unit tests

@jreback
Copy link
Contributor

jreback commented Jul 30, 2015

ok, lgtm. ping when green.

@mortada
Copy link
Contributor Author

mortada commented Jul 30, 2015

sure will do

@@ -1280,12 +1280,29 @@ You can also chain these types of operations:

s.dt.tz_localize('UTC').dt.tz_convert('US/Eastern')

You can also format datetime values as strings with ``Series.dt.strftime`` which
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 use :meth:Series.dt.strftime`` here as well, then it makes the link to the doc page

Copy link
Member

Choose a reason for hiding this comment

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

although that is maybe not that important here as it is exactly the same as standard library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure will do

@mortada mortada force-pushed the dt_strftime branch 2 times, most recently from 8b3456c to c9b9ad8 Compare July 31, 2015 13:23
@mortada
Copy link
Contributor Author

mortada commented Jul 31, 2015

@jorisvandenbossche @jreback updated to return ndarray instead of list

@mortada mortada force-pushed the dt_strftime branch 4 times, most recently from 6c49a7e to b09aacc Compare August 1, 2015 17:34
@mortada
Copy link
Contributor Author

mortada commented Aug 2, 2015

@jorisvandenbossche @jreback it's green now, please take a look

@@ -2362,7 +2362,7 @@ def test_map(self):
f = lambda x: x.strftime('%Y%m%d')
result = rng.map(f)
exp = [f(x) for x in rng]
self.assert_numpy_array_equal(result, exp)
np.testing.assert_array_equal(result, exp)
Copy link
Contributor

Choose a reason for hiding this comment

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

change back. we don't use np.testing.* directly

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 changed it because assert_numpy_array_equal() actually would fail in some of the testing environments. It seems like a problem with the older numpy version (1.8.x)

what should I change it to instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh, you are not comparing numpy arrays (they are actually a list and an array), use exp = np.asarray(....) and I think it will work, you can also try tm.assert_almost_equal(...)

scratch that first part, use tm.assert_almost_equal; I though you were comparing the output of .strftime which was an np.array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah cool I think tm.assert_almost_equal works, thanks! will update shortly

@jreback
Copy link
Contributor

jreback commented Aug 2, 2015

@mortada couple of comments. ping when green.

@mortada
Copy link
Contributor Author

mortada commented Aug 3, 2015

@jreback updated, please take a look

@jreback jreback merged commit 4348781 into pandas-dev:master Aug 3, 2015
@jreback
Copy link
Contributor

jreback commented Aug 3, 2015

thanks!

@mortada mortada deleted the dt_strftime branch August 3, 2015 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Enhancement Output-Formatting __repr__ of pandas objects, to_string Strings String extension data type and string data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: support .strftime for datetimelikes
3 participants