Skip to content

Implement _tz_convert_tzlocal_utc for de-duplication #21727

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

Merged
merged 2 commits into from
Jul 4, 2018

Conversation

jbrockmendel
Copy link
Member

There is a bunch of code in conversion (and a bit in resolution, period, and tslib) that can be de-duplicated. This is one part of ~5 steps in that process.

@@ -1205,7 +1187,6 @@ def is_date_array_normalized(ndarray[int64_t] stamps, tz=None):
for i in range(n):
# Adjust datetime64 timestamp, recompute datetimestruct
pos = trans.searchsorted(stamps[i]) - 1
inf = tz._transition_info[pos]
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated to the rest of the PR

@codecov
Copy link

codecov bot commented Jul 4, 2018

Codecov Report

Merging #21727 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #21727   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files         158      158           
  Lines       49705    49705           
=======================================
  Hits        45693    45693           
  Misses       4012     4012
Flag Coverage Δ
#multiple 90.3% <ø> (ø) ⬆️
#single 41.99% <ø> (ø) ⬆️

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 62a0ebc...6878a69. Read the comment docs.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

minor comments. prefer as a non-private, though if itsonly for this module then private is ok.

@@ -608,32 +608,33 @@ cpdef inline datetime localize_pydatetime(datetime dt, object tz):
# ----------------------------------------------------------------------
# Timezone Conversion

cdef inline int64_t tz_convert_tzlocal_to_utc(int64_t val, tzinfo tz):
cdef inline int64_t _tz_convert_tzlocal_utc(int64_t val, tzinfo tz,
bint to_utc=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good. can you add what this function does in doc-string. and mark as this is private.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mind if I put the docsting in the follow-up? There’s a couple more coming up in the next few hours.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

@jreback jreback added Timezones Timezone data dtype Clean labels Jul 4, 2018
@jreback jreback added this to the 0.24.0 milestone Jul 4, 2018
@jbrockmendel
Copy link
Member Author

Yes, the private function is just for this module.

@jreback jreback merged commit 1070976 into pandas-dev:master Jul 4, 2018
@jbrockmendel jbrockmendel deleted the csimple5 branch July 5, 2018 02:42
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Timezones Timezone data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants