BUG: GH29461 strftime() with nanoseconds for Timestamp #34317

matteosantama · 2020-05-22T15:32:58Z

closes Timestamp.strftime(): missing support for nanoseconds #29461
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

The new method displays %f to the full precision, including trailing zeroes. I could add a check that only displays nanoseconds if they are non-zero.

jreback

pls also add a note in 1.1. bug fix for datetime section

pandas/_libs/tslibs/timestamps.pyx

… compatability with datetime.strptime()

jreback

can you see if we have any benchmarks for these and run them (or add some and run them).

I am afraid this is going to be quite slow.

jreback · 2020-05-22T23:53:13Z

pls also merge master

matteosantama · 2020-05-24T04:38:02Z

Added benchmarks. Looks like it does have a bit of a performance impact.

[100.00%] ··· ====================== =============
                       fmt                        
              ---------------------- -------------
                %Y-%m-%d %H:%M:%S     3.18±0.02μs 
               %Y-%m-%d %H:%M:%S.%f   3.58±0.04μs 
              ====================== =============

       before           after         ratio
     [cb35d8a9]       [2ac690c3]
     <clipboard^2>       <strftime>
+     3.18±0.02μs      4.07±0.02μs     1.28  tslibs.timestamp.TimestampMethods.time_strftime('%Y-%m-%d %H:%M:%S')
+     3.58±0.04μs      4.13±0.02μs     1.15  tslibs.timestamp.TimestampMethods.time_strftime('%Y-%m-%d %H:%M:%S.%f')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.

Weirdly, the performance hit was greater when we DON'T have %f and we call super() (at least in terms of percentage). We shouldn't really be seeing any change in that case. I can't quite figure that one out. I see a few options

Performance impact is not worth the benefit and we scrap the idea
Performance impact is acceptable and we proceed
Can we move this function to Cython? Not totally sure what this would entail

But I think if we can figure out why immediately calling super() is so much slower we'll have a better idea of where we need to make improvements.

jreback · 2020-05-25T22:59:28Z

@matteosantama I am more interested in series.dt.strftime(..) impact here. a tiny hit on Timestamp (likely because its an additional function call) is immaterial. We may or may not have this benchmark.

matteosantama · 2020-05-26T15:56:59Z

Ah, of course. I wrote a benchmark, and it seems that performance has actually increased. I actually don't find this too surprising because strftime() in datetime.py does some unnecessary processing before calling time.strftime(). My implementation calls it directly. I think datetime.strftime() was written before time.strftime() could handle microseconds and timezones.

 before           after         ratio
     [c29e3950]       [f6ba1c9b]
     <master^2>       <strftime>
-       1.86±0.1s       1.67±0.01s     0.90  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S.%f%z', 'NS')
-        769±60ms         679±20ms     0.88  timeseries.DatetimeAccessor.time_dt_accessor_strftime(None, '%Y-%m-%d %H:%M:%S%z', 'NS')
-       1.79±0.1s       1.56±0.03s     0.87  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S.%f%z', 'T')
-       1.74±0.2s       1.49±0.03s     0.86  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S%z', 'S')
-       1.79±0.1s       1.53±0.03s     0.86  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S.%f%z', 'S')
-      1.79±0.08s       1.52±0.02s     0.85  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S%z', 'NS')
-        808±50ms         675±20ms     0.83  timeseries.DatetimeAccessor.time_dt_accessor_strftime(None, '%Y-%m-%d %H:%M:%S.%f%z', 'S')
-        801±30ms         663±10ms     0.83  timeseries.DatetimeAccessor.time_dt_accessor_strftime(None, '%Y-%m-%d %H:%M:%S.%f%z', 'T')
-       1.83±0.1s       1.49±0.03s     0.82  timeseries.DatetimeAccessor.time_dt_accessor_strftime('US/Eastern', '%Y-%m-%d %H:%M:%S%z', 'T')
-        931±10ms         748±10ms     0.80  timeseries.DatetimeAccessor.time_dt_accessor_strftime('UTC', '%Y-%m-%d %H:%M:%S%z', 'NS')
-        946±10ms          759±5ms     0.80  timeseries.DatetimeAccessor.time_dt_accessor_strftime(tzutc(), '%Y-%m-%d %H:%M:%S%z', 'NS')
-        924±20ms          735±3ms     0.79  timeseries.DatetimeAccessor.time_dt_accessor_strftime('UTC', '%Y-%m-%d %H:%M:%S%z', 'T')
-        948±20ms          753±8ms     0.79  timeseries.DatetimeAccessor.time_dt_accessor_strftime(tzutc(), '%Y-%m-%d %H:%M:%S%z', 'S')
-        940±20ms         730±10ms     0.78  timeseries.DatetimeAccessor.time_dt_accessor_strftime('UTC', '%Y-%m-%d %H:%M:%S%z', 'S')
-        964±20ms          744±7ms     0.77  timeseries.DatetimeAccessor.time_dt_accessor_strftime(tzutc(), '%Y-%m-%d %H:%M:%S%z', 'T')
-        979±10ms         754±10ms     0.77  timeseries.DatetimeAccessor.time_dt_accessor_strftime(tzutc(), '%Y-%m-%d %H:%M:%S.%f%z', 'T')
-        989±20ms          758±4ms     0.77  timeseries.DatetimeAccessor.time_dt_accessor_strftime(tzutc(), '%Y-%m-%d %H:%M:%S.%f%z', 'S')
-        971±30ms         739±10ms     0.76  timeseries.DatetimeAccessor.time_dt_accessor_strftime('UTC', '%Y-%m-%d %H:%M:%S.%f%z', 'S')
-        980±30ms          737±7ms     0.75  timeseries.DatetimeAccessor.time_dt_accessor_strftime('UTC', '%Y-%m-%d %H:%M:%S.%f%z', 'T')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

matteosantama · 2020-05-27T18:00:30Z

@jreback CI is failing because of some doc string issues. The current documentation for strftime is pretty useless.

If you think this branch is worth merging, I can update it.

…rftime

WillAyd · 2020-06-07T21:26:07Z

asv_bench/benchmarks/timeseries.py


-    def time_dt_accessor(self, tz):
+    def time_dt_accessor(self, *args):


Can you use explicit parameters here instead of args?

Yeah, I'll fix that.

WillAyd · 2020-06-07T21:27:51Z

pandas/_libs/tslibs/timestamps.pyx

+        # only do additional processing if necessary
+        if self.nanosecond and '%f' in format:
+            newformat = []
+            for ch in format:


Can you not just use a regex here instead?

Yeah, OK that would probably be cleaner

…rftime

matteosantama added 6 commits May 19, 2020 23:24

TST: GH28813 test .diff() on Sparse dtype

12d0b4d

TST: GH28813 test .diff() on Sparse dtype

02c4a85

TST: GH28813 pull sparse diff() test into its own function

7e3256b

Merge branch 'sparse_diff'

f00e48a

Merge branch 'master' of https://github.com/pandas-dev/pandas

a90f928

BUG: GH29461 display nanoseconds with strftime()

26a920c

jreback requested changes May 22, 2020

View reviewed changes

pandas/_libs/tslibs/timestamps.pyx Outdated Show resolved Hide resolved

jreback added Output-Formatting __repr__ of pandas objects, to_string Datetime Datetime data dtype labels May 22, 2020

matteosantama added 5 commits May 22, 2020 16:12

BUG: GH29461 don't display nanoseconds in strftime if none exists for…

3a529ac

… compatability with datetime.strptime()

update whatsnew doc

d0124aa

black formatting

0c0aaf2

remove trailing whitespace to conform to CI

abdbe4e

still trying to pass linting checks

97ae6b3

matteosantama changed the title ~~strftime() with nanoseconds for Timestamp~~ BUG: GH 29461 strftime() with nanoseconds for Timestamp May 22, 2020

matteosantama changed the title ~~BUG: GH 29461 strftime() with nanoseconds for Timestamp~~ BUG: GH29461 strftime() with nanoseconds for Timestamp May 22, 2020

jreback requested changes May 22, 2020

View reviewed changes

matteosantama added 6 commits May 23, 2020 14:44

Merge branch 'master' of https://github.com/pandas-dev/pandas

1132aba

Merge branch 'master' of github.com:matteosantama/pandas into strftime

0e42026

Add strftime benchmarks

ab0c9d4

Early exit strftime if no nanoseconds

532b19c

Make loop more pythonic

88fba5e

Fix benchmark test

2ac690c

Remove whitespace

3b533c9

matteosantama added 2 commits May 26, 2020 02:48

Remove extra function call

f6ba1c9

Benchmark series.strftime()

07b27e2

Merge branch 'master' of https://github.com/pandas-dev/pandas into st…

4b57c4f

…rftime

WillAyd requested changes Jun 7, 2020

View reviewed changes

Matteo Santamaria and others added 9 commits June 8, 2020 17:29

Use explicitly named parameters in testing

d72222a

Use regex for replacing %f

e920d20

Clean up Timestamp._time_repr to use new strftime functionality

34db469

Commiting so I can merge master

fbe286e

Merge branch 'master' of https://github.com/pandas-dev/pandas into st…

54d30eb

…rftime

Use regex for replacing %f

90629c5

Clean up _time_repr to use new strftime functionality

821dfbb

Test for all datetime strftime directives

16b0f9f

Test for all datetime strftime directives

b8565f2

matteosantama closed this Jun 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: GH29461 strftime() with nanoseconds for Timestamp #34317

BUG: GH29461 strftime() with nanoseconds for Timestamp #34317

matteosantama commented May 22, 2020 •

edited

Loading

jreback left a comment

jreback left a comment

jreback commented May 22, 2020

matteosantama commented May 24, 2020

jreback commented May 25, 2020

matteosantama commented May 26, 2020

matteosantama commented May 27, 2020

WillAyd Jun 7, 2020

matteosantama Jun 8, 2020

WillAyd Jun 7, 2020

matteosantama Jun 8, 2020


		def time_dt_accessor(self, tz):
		def time_dt_accessor(self, *args):

BUG: GH29461 strftime() with nanoseconds for Timestamp #34317

BUG: GH29461 strftime() with nanoseconds for Timestamp #34317

Conversation

matteosantama commented May 22, 2020 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

jreback commented May 22, 2020

matteosantama commented May 24, 2020

jreback commented May 25, 2020

matteosantama commented May 26, 2020

matteosantama commented May 27, 2020

WillAyd Jun 7, 2020

Choose a reason for hiding this comment

matteosantama Jun 8, 2020

Choose a reason for hiding this comment

WillAyd Jun 7, 2020

Choose a reason for hiding this comment

matteosantama Jun 8, 2020

Choose a reason for hiding this comment

matteosantama commented May 22, 2020 •

edited

Loading