BUG: Fix parsing of stata dates (#17797) #17990

miker985 · 2017-10-26T16:20:46Z

closes StataReader selectively ignores format details #17797
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
Test and expand parsing of STATA date/time objects.

Expands behavior provided by the following to include most STATA format codes.

        # remove format details from %td
        self.fmtlist = ["%td" if x.startswith("%td") else x
                        for x in self.fmtlist]

Add tests for above behavior (previously untested) + all additional format codes

codecov · 2017-10-26T17:50:17Z

Codecov Report

Merging #17990 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17990      +/-   ##
==========================================
- Coverage   91.23%   91.22%   -0.02%     
==========================================
  Files         163      163              
  Lines       50113    50112       -1     
==========================================
- Hits        45723    45713      -10     
- Misses       4390     4399       +9

Flag	Coverage Δ
#multiple	`89.03% <100%> (-0.01%)`	⬇️
#single	`40.31% <0%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/stata.py	`93.7% <100%> (-0.01%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.75% <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 6779ac0...9ab0681. Read the comment docs.

codecov · 2017-10-26T17:50:28Z

Codecov Report

❗ No coverage uploaded for pull request base (master@cc7abd9). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #17990   +/-   ##
=========================================
  Coverage          ?   91.25%           
=========================================
  Files             ?      163           
  Lines             ?    50099           
  Branches          ?        0           
=========================================
  Hits              ?    45716           
  Misses            ?     4383           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`89.06% <100%> (?)`
#single	`40.24% <0%> (?)`

Impacted Files	Coverage Δ
pandas/io/stata.py	`93.7% <100%> (ø)`

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 cc7abd9...04ec86f. Read the comment docs.

gfyoung · 2017-10-26T18:08:32Z

pandas/tests/io/test_stata.py

+        STATA supports 9 date types which each have distinct units. We test 7
+        of the 9 types, ignoring %tC and %tb. %tC is a variant of %tc that
+        accounts for leap seconds and %tb relies on STATAs business calendar.
+        """


Great description! Can you convert that to a comment block? That's more aligned with how we comment our tests, and also, reference the issue number at the top (instead of at the bottom).

gfyoung · 2017-10-26T18:09:55Z

@miker985 : You're going to need to add a whatsnew for this PR. However, as the doc for 0.21.1 is not available yet, just hold tight. Feel free to ping us later if we forget to let you know.

miker985 · 2017-10-26T18:26:56Z

@gfyoung Is this the comment you're looking for?

I'll wait to hear back on the whatsnew file. If I don't hear back in e.g., a week should I ping you?

gfyoung · 2017-10-26T18:33:43Z

pandas/tests/io/test_stata.py

+        'column', ['ms', 'day', 'week', 'month', 'qtr', 'half', 'yr'])
+    def test_date_parsing_ignores_format_details(self, column):
+        # GH 17797
+        # Test that display formats are ignored when determining if a numeric


Nit: add a newline under the issue reference.

gfyoung · 2017-10-26T18:34:02Z

If I don't hear back in e.g., a week should I ping you?

@TomAugspurger should be merging the PR for the whatsnew soon, so that works.

jreback · 2017-10-27T10:09:53Z

pandas/tests/io/test_stata.py

+        df = read_stata(self.stata_dates)
+        unformatted = df.loc[0, column]
+        formatted = df.loc[0, column + "_fmt"]
+        assert unformatted == formatted


are these supposed to be datetime64[ns] dtype?

what happens for the ignored formats? should raise?

are these supposed to be datetime64[ns] dtype?

At this point in the code formatted and unformatted are pandas._libs.tslib.Timestamp objects. Every column in df has a dtype of datetime64[ns]

what happens for the ignored formats? should raise?

Ignored formats are not converted to dates (consistent with previous behavior) source

jreback · 2017-10-27T10:10:09Z

can you add a whatsnew for 0.21.1 (bug fix io section)

jreback

minor comment. ping on green.

jreback · 2017-10-28T18:51:09Z

doc/source/whatsnew/v0.21.1.txt

@@ -74,6 +74,9 @@ Indexing
 I/O
 ^^^

+- Bug in `StataReader` not converting date/time columns with display formatting addressed (:issue:`17990`)


can you do class:`~pandas.io.stata.StataReader`. Can you make a bit more clear what is being fixed here.

jreback · 2017-10-31T00:24:57Z

thanks @miker985

(cherry picked from commit e886af5)

gfyoung added Bug IO Stata read_stata, to_stata labels Oct 26, 2017

gfyoung reviewed Oct 26, 2017

View reviewed changes

jreback reviewed Oct 27, 2017

View reviewed changes

miker985 added 4 commits October 27, 2017 08:04

BUG: Fix parsing of stata dates (pandas-dev#17797)

b5c2b48

Update comment per @gfyoung comment

bfd520d

Update comment per @gfyoung nit

16cfb87

Add whatsnew note for 0.21.1

b3273c4

miker985 force-pushed the improve-stata-date-loading branch from 2cd0e59 to b3273c4 Compare October 27, 2017 15:14

jreback requested changes Oct 28, 2017

View reviewed changes

jreback added this to the 0.21.1 milestone Oct 28, 2017

jreback added the Needs Backport label Oct 28, 2017

Be clearer about what is being fixed per @jreback

04ec86f

jreback approved these changes Oct 31, 2017

View reviewed changes

jreback merged commit e886af5 into pandas-dev:master Oct 31, 2017

peterpanmj pushed a commit to peterpanmj/pandas that referenced this pull request Oct 31, 2017

BUG: Fix parsing of stata dates (pandas-dev#17797) (pandas-dev#17990)

fc41f3b

miker985 deleted the improve-stata-date-loading branch October 31, 2017 15:03

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

BUG: Fix parsing of stata dates (pandas-dev#17797) (pandas-dev#17990)

bcf8d57

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017

BUG: Fix parsing of stata dates (pandas-dev#17797) (pandas-dev#17990)

6912d25

(cherry picked from commit e886af5)

TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017

BUG: Fix parsing of stata dates (#17797) (#17990)

eb0deaa

(cherry picked from commit e886af5)

TomAugspurger removed the Needs Backport label Dec 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix parsing of stata dates (#17797) #17990

BUG: Fix parsing of stata dates (#17797) #17990

miker985 commented Oct 26, 2017

codecov bot commented Oct 26, 2017

codecov bot commented Oct 26, 2017 •

edited

Loading

gfyoung Oct 26, 2017 •

edited

Loading

gfyoung commented Oct 26, 2017

miker985 commented Oct 26, 2017

gfyoung Oct 26, 2017

gfyoung commented Oct 26, 2017 •

edited

Loading

jreback Oct 27, 2017

miker985 Oct 27, 2017

jreback commented Oct 27, 2017 •

edited

Loading

jreback left a comment

jreback Oct 28, 2017

jreback commented Oct 31, 2017

BUG: Fix parsing of stata dates (#17797) #17990

BUG: Fix parsing of stata dates (#17797) #17990

Conversation

miker985 commented Oct 26, 2017

codecov bot commented Oct 26, 2017

Codecov Report

codecov bot commented Oct 26, 2017 • edited Loading

Codecov Report

gfyoung Oct 26, 2017 • edited Loading

Choose a reason for hiding this comment

gfyoung commented Oct 26, 2017

miker985 commented Oct 26, 2017

gfyoung Oct 26, 2017

Choose a reason for hiding this comment

gfyoung commented Oct 26, 2017 • edited Loading

jreback Oct 27, 2017

Choose a reason for hiding this comment

miker985 Oct 27, 2017

Choose a reason for hiding this comment

jreback commented Oct 27, 2017 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback Oct 28, 2017

Choose a reason for hiding this comment

jreback commented Oct 31, 2017

codecov bot commented Oct 26, 2017 •

edited

Loading

gfyoung Oct 26, 2017 •

edited

Loading

gfyoung commented Oct 26, 2017 •

edited

Loading

jreback commented Oct 27, 2017 •

edited

Loading