-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
BUG: tz aware Timestamp field accessors returns local values (#13303) #15740
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15740 +/- ##
==========================================
- Coverage 91.01% 90.99% -0.02%
==========================================
Files 143 143
Lines 49401 49401
==========================================
- Hits 44961 44952 -9
- Misses 4440 4449 +9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Small comment
val = self.value | ||
if self.tz is not None and not _is_utc(self.tz): | ||
val = tz_convert_single(self.value, 'UTC', self.tz) | ||
out = get_date_field(np.array([val], dtype=np.int64), field) | ||
return int(out[0]) | ||
|
||
cpdef _get_start_end_field(self, field): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't check, but does this _get_start_end_field
needs the same ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is ok, get_date_field handles an i8 only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean with get_date_field
, but this example seems to be wrong:
In [3]: pd.Timestamp("2014-01-01 00:00:00+01:00").is_month_start
Out[3]: False
In [4]: pd.Timestamp("2014-01-01 00:00:00").is_month_start
Out[4]: True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh sorry, was looking at something else. get_start_end_field
looks to need fixing as well. thanks @jorisvandenbossche
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, get_date_field
was on three lines above
can you add similar type tests for |
…dev#13303) Add whatsnew Add tests for datetimeindex and fix Timestamp start/end attributes
lgtm. |
Thanks @jreback! |
…dev#13303) closes pandas-dev#13303 Previously, calling a date/time attribute with Timestamp that's tz aware (e.g. `Timestamp('...', tz='...').dayofyear`) would return the attribute in UTC instead of the local tz. Author: Matt Roeschke <[email protected]> Closes pandas-dev#15740 from mroeschke/fix_13303 and squashes the following commits: b78b333 [Matt Roeschke] BUG: tz aware Timestamp field accessors returns local values (pandas-dev#13303)
…dev#13303) closes pandas-dev#13303 Previously, calling a date/time attribute with Timestamp that's tz aware (e.g. `Timestamp('...', tz='...').dayofyear`) would return the attribute in UTC instead of the local tz. Author: Matt Roeschke <[email protected]> Closes pandas-dev#15740 from mroeschke/fix_13303 and squashes the following commits: b78b333 [Matt Roeschke] BUG: tz aware Timestamp field accessors returns local values (pandas-dev#13303)
git diff upstream/master | flake8 --diff
Previously, calling a date/time attribute with Timestamp that's tz aware (e.g.
Timestamp('...', tz='...').dayofyear
) would return the attribute in UTC instead of the local tz.