BUG: Return local Timestamp.weekday_name attribute (#17354) #17377

mroeschke · 2017-08-30T06:28:00Z

closes BUG: Timestamp.weekday_name uses UTC instead of given timezone #17354
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

@jreback created a new method _get_named_field based on your comment in the issue thread.

jreback · 2017-08-30T10:26:09Z

doc/source/whatsnew/v0.21.0.txt

@@ -342,6 +342,7 @@ Conversion
 - Fixed the return type of ``IntervalIndex.is_non_overlapping_monotonic`` to be a Python ``bool`` for consistency with similar attributes/methods.  Previously returned a ``numpy.bool_``. (:issue:`17237`)
 - Bug in ``IntervalIndex.is_non_overlapping_monotonic`` when intervals are closed on both sides and overlap at a point (:issue:`16560`)
 - Bug in :func:`Series.fillna` returns frame when ``inplace=True`` and ``value`` is dict (:issue:`16156`)
+- Bug in ``Timestamp.weekday_name`` returning a UTC-based weekday name when localized to a timezone (:issue:`17354`)


you can do :func:`Timestamp.weekday_name` I think (might need to be attr actually)

jreback · 2017-08-30T10:27:00Z

pandas/_libs/tslib.pyx

@@ -1268,13 +1266,23 @@ cdef class _Timestamp(datetime):
        # same timezone if specified)
        return datetime.__sub__(self, other)

-    cpdef _get_field(self, field):
+    cpdef _maybe_convert_value_to_local(self):


this can be just cdef and should return int64_t

jreback · 2017-08-30T10:27:34Z

pandas/_libs/tslib.pyx

+        return val
+
+    cpdef _get_field(self, field):
+        val = self._maybe_convert_value_to_local()


you can type val as cdef: int64_t (same for _get_named_field)

codecov · 2017-09-01T20:54:01Z

Codecov Report

Merging #17377 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17377      +/-   ##
==========================================
- Coverage   91.16%   91.14%   -0.02%     
==========================================
  Files         163      163              
  Lines       49581    49581              
==========================================
- Hits        45199    45190       -9     
- Misses       4382     4391       +9

Flag	Coverage Δ
#multiple	`88.92% <ø> (ø)`	⬆️
#single	`40.25% <ø> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.72% <0%> (-0.1%)`	⬇️

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 8351f86...18852f4. Read the comment docs.

codecov · 2017-09-01T20:54:01Z

Codecov Report

Merging #17377 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17377      +/-   ##
==========================================
- Coverage   91.16%   91.14%   -0.03%     
==========================================
  Files         163      163              
  Lines       49581    49587       +6     
==========================================
- Hits        45199    45194       -5     
- Misses       4382     4393      +11

Flag	Coverage Δ
#multiple	`88.92% <ø> (-0.01%)`	⬇️
#single	`40.25% <ø> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/io/parquet.py	`65.38% <0%> (-0.37%)`	⬇️
pandas/core/frame.py	`97.72% <0%> (-0.1%)`	⬇️
pandas/core/indexes/base.py	`96.29% <0%> (ø)`	⬆️
pandas/core/indexes/multi.py	`96.9% <0%> (ø)`	⬆️
pandas/plotting/_core.py	`82.69% <0%> (+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 8351f86...ddaa4e3. Read the comment docs.

mroeschke · 2017-09-05T05:04:23Z

cdef'd _maybe_convert_value_to_local and val argument in related function and all green.

jreback

some cdef comments. lgtm. ping when changed / pushed.

jreback · 2017-09-05T10:20:48Z

pandas/_libs/tslib.pyx

@@ -1268,13 +1266,27 @@ cdef class _Timestamp(datetime):
        # same timezone if specified)
        return datetime.__sub__(self, other)

-    cpdef _get_field(self, field):
+    cdef int64_t _maybe_convert_value_to_local(self):
+        """Convert UTC i8 value to local i8 value if tz exists"""


you need to add a

cdef int64_t val

here in order to type it

jreback · 2017-09-05T10:21:08Z

pandas/_libs/tslib.pyx

+        return val
+
+    cpdef _get_field(self, field):
+        cdef:


jreback · 2017-09-05T10:22:10Z

pandas/_libs/tslib.pyx

+    cpdef _get_field(self, field):
+        cdef:
+            int64_t val
+        val = self._maybe_convert_value_to_local()


if you can type out in these (ndarray[int32_t]) i think

jreback · 2017-09-06T11:53:04Z

pandas/_libs/tslib.pyx

+            int64_t val
+            ndarray[int64_t] date_array = np.empty(1, dtype=np.int64)
+        val = self._maybe_convert_value_to_local()
+        date_array[0] = val


this is overkill to type the input ndarray itself

it was out which was looking to type, though not a big deal, so just remove the date_array temp. ping on green.

jreback · 2017-09-07T00:46:53Z

thanks!

…pandas-dev#17377)

jreback added Bug Datetime Datetime data dtype Timezones Timezone data dtype labels Aug 30, 2017

jreback requested changes Aug 30, 2017

View reviewed changes

mroeschke force-pushed the fix_17354 branch from 98d2d97 to 14c0fce Compare September 1, 2017 04:52

mroeschke added 4 commits September 1, 2017 12:10

BUG: Return local Timestamp.weekday_name attribute (pandas-dev#17354)

120e12d

Add whatsnew

fb4095a

Type ctype variables and functions, whatsnew fix

e453d10

Balance triple quotes

18852f4

mroeschke force-pushed the fix_17354 branch from 14c0fce to 18852f4 Compare September 1, 2017 19:11

jreback requested changes Sep 5, 2017

View reviewed changes

jreback added this to the 0.21.0 milestone Sep 5, 2017

Ctype array and leftover val

9d0031f

jreback reviewed Sep 6, 2017

View reviewed changes

untype array

ddaa4e3

jreback approved these changes Sep 7, 2017

View reviewed changes

jreback merged commit 3a12687 into pandas-dev:master Sep 7, 2017

mroeschke mentioned this pull request Sep 7, 2017

cdef out dtype for _Timestamp._get_field #17457

Merged

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Sep 10, 2017

BUG: Return local Timestamp.weekday_name attribute (pandas-dev#17354) (…

991af8f

…pandas-dev#17377)

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

BUG: Return local Timestamp.weekday_name attribute (pandas-dev#17354) (…

a39f0f1

…pandas-dev#17377)

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

BUG: Return local Timestamp.weekday_name attribute (pandas-dev#17354) (…

0d18ab1

…pandas-dev#17377)

mroeschke deleted the fix_17354 branch December 20, 2017 02:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Return local Timestamp.weekday_name attribute (#17354) #17377

BUG: Return local Timestamp.weekday_name attribute (#17354) #17377

mroeschke commented Aug 30, 2017

jreback Aug 30, 2017

jreback Aug 30, 2017

jreback Aug 30, 2017

codecov bot commented Sep 1, 2017

codecov bot commented Sep 1, 2017 •

edited

Loading

mroeschke commented Sep 5, 2017

jreback left a comment

jreback Sep 5, 2017

jreback Sep 5, 2017

jreback Sep 5, 2017

jreback Sep 6, 2017

jreback commented Sep 7, 2017

BUG: Return local Timestamp.weekday_name attribute (#17354) #17377

BUG: Return local Timestamp.weekday_name attribute (#17354) #17377

Conversation

mroeschke commented Aug 30, 2017

jreback Aug 30, 2017

Choose a reason for hiding this comment

jreback Aug 30, 2017

Choose a reason for hiding this comment

jreback Aug 30, 2017

Choose a reason for hiding this comment

codecov bot commented Sep 1, 2017

Codecov Report

codecov bot commented Sep 1, 2017 • edited Loading

Codecov Report

mroeschke commented Sep 5, 2017

jreback left a comment

Choose a reason for hiding this comment

jreback Sep 5, 2017

Choose a reason for hiding this comment

jreback Sep 5, 2017

Choose a reason for hiding this comment

jreback Sep 5, 2017

Choose a reason for hiding this comment

jreback Sep 6, 2017

Choose a reason for hiding this comment

jreback commented Sep 7, 2017

codecov bot commented Sep 1, 2017 •

edited

Loading