PERF: Datetime/Timestamp.normalize for timezone naive datetimes #23634

mroeschke · 2018-11-12T04:32:11Z

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

       before           after         ratio
     [bb32564c]       [ff171f75]
-         281±2μs          197±2μs     0.70  timeseries.DatetimeIndex.time_normalize('dst')
-     4.26±0.05ms      2.64±0.04ms     0.62  timeseries.DatetimeAccessor.time_dt_accessor_normalize
-     3.78±0.01ms      2.28±0.01ms     0.60  timeseries.DatetimeIndex.time_normalize('repeated')
-     3.79±0.05ms      2.23±0.06ms     0.59  timeseries.DatetimeIndex.time_normalize('tz_naive')


       before           after         ratio
     [bb32564c]       [ff171f75]
-      11.9±0.9μs         1.56±0μs     0.13  timestamp.TimestampOps.time_normalize(None)

pep8speaks · 2018-11-12T04:32:15Z

Hello @mroeschke! Thanks for submitting the PR.

There are no PEP8 issues in the file asv_bench/benchmarks/timestamp.py !
There are no PEP8 issues in the file pandas/core/arrays/datetimes.py !
There are no PEP8 issues in the file pandas/tests/indexes/datetimes/test_scalar_compat.py !
There are no PEP8 issues in the file pandas/tests/scalar/timestamp/test_unary_ops.py !

jbrockmendel · 2018-11-12T05:10:23Z

pandas/_libs/tslibs/conversion.pyx

-    if tz is not None:
-        tz = maybe_get_tz(tz)
-        result = _normalize_local(stamps, tz)
-    else:


Is this case never reached?

Correct. This case (the naive case) is handled in these two places now:

https://github.com/pandas-dev/pandas/pull/23634/files#diff-231ac35d2116a12844a7cfed02730580R1289
https://github.com/pandas-dev/pandas/pull/23634/files#diff-960da60b33d858481ad8799b0e84764bR835

codecov · 2018-11-12T05:17:41Z

Codecov Report

Merging #23634 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #23634      +/-   ##
==========================================
+ Coverage   92.23%   92.23%   +<.01%     
==========================================
  Files         161      161              
  Lines       51408    51414       +6     
==========================================
+ Hits        47416    47422       +6     
  Misses       3992     3992

Flag	Coverage Δ
#multiple	`90.62% <100%> (ø)`	⬆️
#single	`42.29% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/arrays/datetimes.py	`98.5% <100%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24bce1a...fb11dcf. Read the comment docs.

jreback · 2018-11-12T13:10:51Z

pandas/_libs/tslibs/timestamps.pyx

@@ -40,6 +40,7 @@ from timezones cimport (
 # Constants
 _zero_time = datetime_time(0, 0)
 _no_input = object()
+cdef int64_t DAY_NS = 86400000000000


we have DAY_NS defined in lots of places, can you move to 1

(bamboo-dev) jreback@dev:~/pandas-dev$ grep -r 86400 pandas/_libs/ --include '*.pyx' pandas/_libs/tslibs/period.pyx: {1, 24, 1440, 86400, 86400000, 86400000000, 86400000000000}, pandas/_libs/tslibs/period.pyx: seconds = unix_date * 86400 + dts.hour * 3600 + dts.min * 60 + dts.sec pandas/_libs/tslibs/period.pyx: abstime += 86400 pandas/_libs/tslibs/period.pyx: while abstime >= 86400: pandas/_libs/tslibs/period.pyx: abstime -= 86400 pandas/_libs/tslibs/period.pyx: # abstime >= 0.0 and abstime <= 86400 pandas/_libs/tslibs/conversion.pyx:cdef int64_t DAY_NS = 86400000000000LL pandas/_libs/tslibs/timedeltas.pyx:cdef int64_t DAY_NS = 86400000000000LL pandas/_libs/tslibs/timedeltas.pyx: m = 1000000000L * 86400 * 7 pandas/_libs/tslibs/timedeltas.pyx: m = 1000000000L * 86400 pandas/_libs/tslibs/timedeltas.pyx: 86400000000042 pandas/_libs/tslibs/fields.pyx: micros = np.mod(dtindex, 86400000000000, dtype=np.int64) // 1000LL pandas/_libs/tslibs/src/datetime/np_datetime.c: npy_int64 DAY_NS = 86400000000000LL;

prob should be in np_datetime.pyx (and import from there)

can you move to the same place you have DAY_SECONDS

jreback · 2018-11-12T13:12:09Z

pandas/tests/scalar/timestamp/test_unary_ops.py

+    # --------------------------------------------------------------
+    # Timestamp.normalize
+
+    @pytest.mark.parametrize('arg', ['2013-11-30', '2013-11-30 12:00:00'])


is there a normalize_nat test as well?

We don't define normalize for NaT.

We could have one for Timstamp mirroring (another issue). Probably would just return NaT

…ance

mroeschke · 2018-11-13T21:33:48Z

@jreback gathered some of the DAY_NS usage into a constant in np_datetime.pyx and all green.

jreback · 2018-11-14T13:59:17Z

pandas/_libs/tslibs/conversion.pyx

@@ -41,7 +42,6 @@ from nattype cimport NPY_NAT, checknull_with_nat
 # ----------------------------------------------------------------------
 # Constants

-cdef int64_t DAY_NS = 86400000000000LL
 cdef int64_t HOURS_NS = 3600000000000


should prob move this one too (future ok)

can you move this one as well

jreback · 2018-11-14T14:00:05Z

pandas/_libs/tslibs/conversion.pyx

@@ -22,7 +22,8 @@ from np_datetime cimport (check_dts_bounds,
                          npy_datetime,
                          dt64_to_dtstruct, dtstruct_to_dt64,
                          get_datetime64_unit, get_datetime64_value,
-                          pydatetime_to_dt64, NPY_DATETIMEUNIT, NPY_FR_ns)
+                          pydatetime_to_dt64, NPY_DATETIMEUNIT, NPY_FR_ns,


what is DAY_S, you mean DAY_NS right? let's write out these constants.

jreback · 2018-11-14T14:00:28Z

pandas/_libs/tslibs/np_datetime.pyx

+# ----------------------------------------------------------------------
+# time constants
+
+cdef int64_t DAY_S = 86400


let's write this out to DAY_SECONDS

I think the place for these may be ccalendar

jreback · 2018-11-14T14:00:52Z

pandas/_libs/tslibs/timestamps.pyx

@@ -40,6 +40,7 @@ from timezones cimport (
 # Constants
 _zero_time = datetime_time(0, 0)
 _no_input = object()
+cdef int64_t DAY_NS = 86400000000000


can you move to the same place you have DAY_SECONDS

jreback · 2018-11-14T14:01:01Z

pandas/core/arrays/datetimes.py

@@ -832,7 +832,14 @@ def normalize(self):
                       '2014-08-01 00:00:00+05:30'],
                       dtype='datetime64[ns, Asia/Calcutta]', freq=None)
        """
-        new_values = conversion.normalize_i8_timestamps(self.asi8, self.tz)
+        if self.tz is None:
+            not_null = self.notnull()


should this be notna? (does DatetimeArray even have notna or notnull?)

It (DatetimeIndex) apparently has notnull, but not sure if i should be using notna or notnull

can you use notna

…ance

mroeschke · 2018-11-16T00:38:53Z

All green after some flaky azure tests.

jreback · 2018-11-16T17:57:04Z

pandas/_libs/tslibs/conversion.pyx

@@ -41,7 +42,6 @@ from nattype cimport NPY_NAT, checknull_with_nat
 # ----------------------------------------------------------------------
 # Constants

-cdef int64_t DAY_NS = 86400000000000LL
 cdef int64_t HOURS_NS = 3600000000000


can you move this one as well

…ance

jreback

tiny change. ping on green.

jreback · 2018-11-17T23:30:20Z

pandas/core/arrays/datetimes.py

@@ -832,7 +832,14 @@ def normalize(self):
                       '2014-08-01 00:00:00+05:30'],
                       dtype='datetime64[ns, Asia/Calcutta]', freq=None)
        """
-        new_values = conversion.normalize_i8_timestamps(self.asi8, self.tz)
+        if self.tz is None:
+            not_null = self.notnull()


can you use notna

…ance

mroeschke · 2018-11-18T03:06:57Z

Ping all green @jreback

jreback · 2018-11-18T18:27:41Z

thanks @mroeschke

…fixed * upstream/master: (46 commits) DEPS: bump xlrd min version to 1.0.0 (pandas-dev#23774) BUG: Don't warn if default conflicts with dialect (pandas-dev#23775) BUG: Fixing memory leaks in read_csv (pandas-dev#23072) TST: Extend datetime64 arith tests to array classes, fix several broken cases (pandas-dev#23771) STYLE: Specify bare exceptions in pandas/tests (pandas-dev#23370) ENH: between_time, at_time accept axis parameter (pandas-dev#21799) PERF: Use is_utc check to improve performance of dateutil UTC in DatetimeIndex methods (pandas-dev#23772) CLN: io/formats/html.py: refactor (pandas-dev#22726) API: Make Categorical.searchsorted returns a scalar when supplied a scalar (pandas-dev#23466) TST: Add test case for GH14080 for overflow exception (pandas-dev#23762) BUG: Don't extract header names if none specified (pandas-dev#23703) BUG: Index.str.partition not nan-safe (pandas-dev#23558) (pandas-dev#23618) DEPR: tz_convert in the Timestamp constructor (pandas-dev#23621) PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pandas-dev#23634) TST: Use new arithmetic fixtures, parametrize many more tests (pandas-dev#23757) REF/TST: Add more pytest idiom to parsers tests (pandas-dev#23761) DOC: Add ignore-deprecate argument to validate_docstrings.py (pandas-dev#23650) ENH: update pandas-gbq to 0.8.0, adds credentials arg (pandas-dev#23662) DOC: Improve error message to show correct order (pandas-dev#23652) ENH: Improve error message for empty object array (pandas-dev#23718) ...

…as-dev#23634) \

Matt Roeschke added 4 commits November 11, 2018 16:59

PERF: Timestamp.normalize

62827ef

PERF DatetimeIndex.normalize

ff171f7

remove unused cython code

efb281f

remove more unused cython

cc6eee0

Add whatsnew

b90abd9

jbrockmendel reviewed Nov 12, 2018

View reviewed changes

gfyoung added Datetime Datetime data dtype Performance Memory or execution speed performance Internals Related to non-user accessible pandas implementation Clean labels Nov 12, 2018

typo

35fac22

jreback requested changes Nov 12, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Nov 12, 2018

Matt Roeschke added 3 commits November 12, 2018 15:22

Merge remote-tracking branch 'upstream/master' into normalize_perform…

f09559a

…ance

collect day_ns useage into day_s

0b3a664

Merge remote-tracking branch 'upstream/master' into normalize_perform…

9042aa1

…ance

jreback requested changes Nov 14, 2018

View reviewed changes

Matt Roeschke added 5 commits November 14, 2018 11:40

Merge remote-tracking branch 'upstream/master' into normalize_perform…

3a23170

…ance

Move constant to ccalendar

52a7eb2

Merge remote-tracking branch 'upstream/master' into normalize_perform…

6204d21

…ance

Merge remote-tracking branch 'upstream/master' into normalize_perform…

1f1d455

…ance

Merge remote-tracking branch 'upstream/master' into normalize_perform…

44a8808

…ance

jreback approved these changes Nov 16, 2018

View reviewed changes

jreback requested changes Nov 16, 2018

View reviewed changes

Matt Roeschke added 2 commits November 16, 2018 10:04

Merge remote-tracking branch 'upstream/master' into normalize_perform…

e6c74d2

…ance

move HOUR_NS

0ece208

Add performance for UTC timestamps

bc5571a

jreback requested changes Nov 17, 2018

View reviewed changes

Matt Roeschke added 2 commits November 17, 2018 16:54

Merge remote-tracking branch 'upstream/master' into normalize_perform…

243d73a

…ance

use notna instead

fb11dcf

jreback approved these changes Nov 18, 2018

View reviewed changes

jreback merged commit 3702de2 into pandas-dev:master Nov 18, 2018

mroeschke deleted the normalize_performance branch November 18, 2018 18:29

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pand…

46d3891

…as-dev#23634) \

mroeschke mentioned this pull request Dec 23, 2018

CLN: tslibs imports and unused variables #24401

Merged

1 task

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pand…

e774c0f

…as-dev#23634) \

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pand…

1ffd5f4

…as-dev#23634) \

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERF: Datetime/Timestamp.normalize for timezone naive datetimes #23634

PERF: Datetime/Timestamp.normalize for timezone naive datetimes #23634

mroeschke commented Nov 12, 2018

pep8speaks commented Nov 12, 2018

jbrockmendel Nov 12, 2018

mroeschke Nov 12, 2018

codecov bot commented Nov 12, 2018 •

edited

Loading

jreback Nov 12, 2018

jreback Nov 14, 2018

jreback Nov 12, 2018

mroeschke Nov 12, 2018

mroeschke Nov 12, 2018

mroeschke commented Nov 13, 2018

jreback Nov 14, 2018

jreback Nov 16, 2018

jreback Nov 14, 2018

jreback Nov 14, 2018

jbrockmendel Nov 14, 2018

jreback Nov 14, 2018

jreback Nov 14, 2018

jbrockmendel Nov 14, 2018

mroeschke Nov 14, 2018

jreback Nov 17, 2018

mroeschke commented Nov 16, 2018

jreback Nov 16, 2018

jreback left a comment

jreback Nov 17, 2018

mroeschke commented Nov 18, 2018

jreback commented Nov 18, 2018

PERF: Datetime/Timestamp.normalize for timezone naive datetimes #23634

PERF: Datetime/Timestamp.normalize for timezone naive datetimes #23634

Conversation

mroeschke commented Nov 12, 2018

pep8speaks commented Nov 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 12, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Nov 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Nov 16, 2018

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Nov 18, 2018

jreback commented Nov 18, 2018

codecov bot commented Nov 12, 2018 •

edited

Loading