ENH: to_datetime support iso week year (16607) #16661

rosygupta · 2017-06-10T21:55:09Z

ENH: to_datetime support iso week year

closes to_datetime should support ISO week year #16607
test added / passed
passes git diff upstream/master --name-only -- '*.py' | flake8 --diff
whatsnew entry

ENH: to_datetime support iso week year

…-rosy

gfyoung · 2017-06-11T09:19:27Z

@rosygupta : Thanks for the contribution! A couple of things:

Instead of putting "yes", an "x" between the brackets will suffice, as it will render as a check-mark.
Your tests will need to be more comprehensive. For starters, you should write additional tests that hit each of those ValueError messages that you added.

rosygupta · 2017-06-11T09:20:59Z

@gfyoung thanks for reviewing the PR. Is it not possible to create a PR without the tests? Also, my PR failed the build yesterday. Could you help me figure out the reason?

rosygupta · 2017-06-11T09:25:16Z

@gfyoung how can I squash all my commits post creating the PR. I realise its better to have a single commit in the PR instead of multiple? Should I close this PR and open a new one with another branch?

gfyoung · 2017-06-11T09:43:48Z

Is it not possible to create a PR without the tests?

Not sure what you mean by this. Could you clarify?

Also, my PR failed the build yesterday. Could you help me figure out the reason?

Why don't we wait until your builds right now finish, and I can look.

how can I squash all my commits post creating the PR. I realise its better to have a single commit in the PR instead of multiple? Should I close this PR and open a new one with another branch?

Aesthetically, that is true. However, I wouldn't worry too much. When maintainers merge, they will squash for you. However, if you're interested, this SO link should help.

And NO, do not close this PR (in general, it is always better to clean up what you've already created than it is to start from scratch).

rosygupta · 2017-06-11T09:46:40Z

@gfyoung Thanks for clarifying. I mean this PR will be accepted only if the tests that you recommended me to write are pushed here?

gfyoung · 2017-06-11T09:50:32Z

I mean this PR will be accepted only if the tests that you recommended me to write are pushed here?

"only if" is a little strong, but at this point, those would certainly be tests I would add in order to get it closer to merge-ability.

rosygupta · 2017-06-11T09:54:14Z

@gfyoung that clears the doubt. The build just failed. If you could look into it.

jreback · 2017-06-11T15:55:38Z

pandas/_libs/tslib.pyx

+    ordinal = (iso_week * 7) + iso_weekday - correction
+    # ordinal may be negative or 0 now, which means the date is in the previous
+    # calendar year
+    if ordinal < 1:


so want to have a test case that exercises both of these clauses

can you mark which test case this is below with a comment

@pytest.mark.parametrize("s, _format, dt", [

['2015-1-1', '%G-%V-%u', datetime(2014, 12, 29, 0, 0)],

['2015-1-4', '%G-%V-%u', datetime(2015, 1, 1, 0, 0)],

['2015-1-7', '%G-%V-%u', datetime(2015, 1, 4, 0, 0)]

])----- out of these;

['2015-1-4', '%G-%V-%u', datetime(2015, 1, 1, 0, 0)],

this one tests this case!

jreback · 2017-06-11T15:56:36Z

pandas/tests/indexes/datetimes/test_tools.py

@@ -175,6 +175,14 @@ def test_to_datetime_format_weeks(self):

 class TestToDatetime(object):

+    def test_to_datetime_iso_week_year_format(self):


do these cases separately.

need tests for each of the ValueErrors that you are raising

as I indicated above, need to exercise the paths in the code.

@jreback Can you please clarify this--> as I indicated above, need to exercise the paths in the code.?

#16661 (comment)

I mean exactly this. you have an if clause so we need a test for both cases.

ordinal may be negative or 0 now, which means the date is in the previous calendar year

gfyoung · 2017-06-11T18:19:55Z

The build just failed. If you could look into it.

Not entirely why the clipboard test is failing. Seems like a spurious failure @jreback ?

ENH: added tests for each of the ValueError, improved checks

gfyoung · 2017-06-11T21:59:26Z

pandas/tests/indexes/datetimes/test_tools.py

+    def test_ValueError_iso_week_year(self):
+        # 1. ISO week (%V) is specified, but the year is specified with %Y
+        # instead of %G
+        with pytest.raises(ValueError):


Let's use tm.assert_raises_regex instead. It is stronger to test not only the Exception raised but also the message that comes along with it.

gfyoung · 2017-06-11T22:05:27Z

pandas/tests/indexes/datetimes/test_tools.py

@@ -175,6 +175,42 @@ def test_to_datetime_format_weeks(self):

 class TestToDatetime(object):

+    def test_to_datetime_iso_week_year_format(self):
+        data = [


Let's parametrize this using pytest:

@pytest.mark.parametrize(...) def test_to_datetime_iso_week_year_format(s, _format, dt): ...

Also note that I use "_format" instead of "format" because as a personal preference, I prefer not to use keywords when creating local variables.

codecov · 2017-06-12T20:42:48Z

Codecov Report

Merging #16661 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #16661      +/-   ##
==========================================
+ Coverage   90.93%   90.93%   +<.01%     
==========================================
  Files         161      161              
  Lines       49266    49269       +3     
==========================================
+ Hits        44801    44804       +3     
  Misses       4465     4465

Flag	Coverage Δ
#multiple	`88.69% <ø> (ø)`	⬆️
#single	`40.22% <ø> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/generic.py	`92.27% <0%> (-0.1%)`	⬇️
pandas/core/tools/datetimes.py	`67.5% <0%> (+0.55%)`	⬆️

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 ceaf852...89eaf1a. Read the comment docs.

codecov · 2017-06-12T20:43:03Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #16661      +/-   ##
==========================================
- Coverage   90.93%   90.92%   -0.02%     
==========================================
  Files         161      161              
  Lines       49266    49284      +18     
==========================================
+ Hits        44801    44811      +10     
- Misses       4465     4473       +8

Flag	Coverage Δ
#multiple	`88.69% <ø> (ø)`	⬆️
#single	`40.17% <ø> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/pytables.py	`92.87% <0%> (-0.25%)`	⬇️
pandas/plotting/_core.py	`81.83% <0%> (-0.1%)`	⬇️
pandas/core/generic.py	`92.27% <0%> (-0.09%)`	⬇️
pandas/io/excel.py	`80.55% <0%> (ø)`	⬆️
pandas/io/parsers.py	`95.43% <0%> (ø)`	⬆️
pandas/core/resample.py	`96.13% <0%> (+0.03%)`	⬆️
pandas/core/tools/datetimes.py	`67.5% <0%> (+0.55%)`	⬆️

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 ceaf852...c249f66. Read the comment docs.

rosygupta · 2017-06-13T05:55:49Z

@gfyoung @jreback Any clue why the build is failing?

gfyoung · 2017-06-13T06:10:39Z

On Travis, you have style-check errors. Code will not be merged unless it conforms with the Python style guide as per PEP8 style guidelines. You can check yourself by install flake8 and running the check listed in the PR template.

On Circle, it appears that your error messages don't match the ones being raised, which is a little weird given that they passed on Appveyor and Travis. However, you should still try to figure out, if possible, why the test you're testing for doesn't work.

autoindented Test File

build fail fix

jreback · 2017-06-20T10:32:02Z

pandas/tests/indexes/datetimes/test_tools.py

-                       dtype=object)
+        arr = np.array(
+            [
+                us_eastern.localize(


don't change unrelated things

jreback

pls add a release note. 0.21.0 in enhancements.

jreback · 2017-06-20T10:34:34Z

pandas/_libs/tslib.pyx

@@ -3701,13 +3704,14 @@ def array_strptime(ndarray[object] values, object fmt,
                raise ValueError("time data %r does not match format "
                                 "%r (search)" % (values[i], fmt))

+        iso_year = -1


these need to be defined above in the cdef (iso_year/iso_week)

jreback · 2017-07-19T10:32:30Z

can you rebase / update according to comments

pep8speaks · 2017-07-19T19:36:55Z

Hello @rosygupta! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 19, 2017 at 20:15 Hours UTC

gfyoung · 2017-07-21T15:16:55Z

@rosygupta : These changes LGTM. Just need to resolve the merge conflict, and it should be on its way!

jreback

small comment on the whatsnew note. too bad this: #16669 is not done, otherwise would have a place to add a version added note.

jreback · 2017-07-21T23:28:24Z

doc/source/whatsnew/v0.21.0.txt

@@ -24,6 +24,9 @@ New features
  <https://www.python.org/dev/peps/pep-0519/>`_ on most readers and writers (:issue:`13823`)
 - Added `__fspath__` method to :class`:pandas.HDFStore`, :class:`pandas.ExcelFile`,
  and :class:`pandas.ExcelWriter` to work properly with the file system path protocol (:issue:`13823`)
+- Added 'iso week year support to to_datetime', added method '_calc_julian_from_V'


this is not clear. more like, added support for %G strptime directive in parsing datetimes; no need to mention the second part which is purely internal.

jreback · 2017-09-23T16:53:18Z

IIRC this was pretty reasonable. can you rebase / update to comments?

jreback · 2017-10-28T00:34:08Z

can you rebase

jreback · 2017-11-23T16:45:30Z

closing as this is stale.

rosygupta added 4 commits June 11, 2017 02:49

GH16607

274a8e0

ENH: to_datetime support iso week year

print removed

7b622ed

GH16607

437dea9

ENH: to_datetime support iso week year

Merge branch 'pandas-rosy' of github.com:rosygupta/pandas into pandas…

e349ccc

…-rosy

rosygupta closed this Jun 11, 2017

rosygupta deleted the pandas-rosy branch June 11, 2017 09:13

rosygupta restored the pandas-rosy branch June 11, 2017 09:15

rosygupta reopened this Jun 11, 2017

rosygupta changed the title ~~Pandas rosy~~ ENH: to_datetime support iso week year Jun 11, 2017

rosygupta changed the title ~~ENH: to_datetime support iso week year~~ ENH: to_datetime support iso week year (16607) Jun 11, 2017

jreback added Compat pandas objects compatability with Numpy or Python functions Datetime Datetime data dtype labels Jun 11, 2017

jreback requested changes Jun 11, 2017

View reviewed changes

GH16607

9f53a99

ENH: added tests for each of the ValueError, improved checks

gfyoung reviewed Jun 11, 2017

View reviewed changes

tests added with tm.assert_raises_regex

89eaf1a

GH16607

aff309c

autoindented Test File

GH 16607

71a1c13

build fail fix

jreback reviewed Jun 20, 2017

View reviewed changes

jreback requested changes Jun 20, 2017

View reviewed changes

jreback mentioned this pull request Jun 28, 2017

to_datetime parses dates incorrectly when the format includes '%W' and does not include day of week plus calendar year #16774

Closed

TomAugspurger added this to the 0.21.0 milestone Jun 30, 2017

removed pep styling from unnecessary code

71bc348

rosygupta added 6 commits July 20, 2017 01:08

removed pep styling from unnecessary code

028e978

removed pep styling from unnecessary code

cf58798

removed pep styling from unnecessary code

137b724

removed pep styling from unnecessary code

250e2cc

added variables in cdef

652bb90

added in release note

c249f66

gfyoung approved these changes Jul 21, 2017

View reviewed changes

jreback requested changes Jul 21, 2017

View reviewed changes

jreback removed this from the 0.21.0 milestone Sep 23, 2017

jreback closed this Nov 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: to_datetime support iso week year (16607) #16661

ENH: to_datetime support iso week year (16607) #16661

rosygupta commented Jun 10, 2017 •

edited

Loading

gfyoung commented Jun 11, 2017

rosygupta commented Jun 11, 2017

rosygupta commented Jun 11, 2017

gfyoung commented Jun 11, 2017 •

edited

Loading

rosygupta commented Jun 11, 2017 •

edited

Loading

gfyoung commented Jun 11, 2017

rosygupta commented Jun 11, 2017

jreback Jun 11, 2017

jreback Jun 20, 2017

rosygupta Jul 19, 2017

jreback Jun 11, 2017

rosygupta Jun 11, 2017

jreback Jun 11, 2017

gfyoung commented Jun 11, 2017

gfyoung Jun 11, 2017

gfyoung Jun 11, 2017

codecov bot commented Jun 12, 2017

codecov bot commented Jun 12, 2017 •

edited

Loading

rosygupta commented Jun 13, 2017

gfyoung commented Jun 13, 2017 •

edited

Loading

jreback Jun 20, 2017

jreback left a comment

jreback Jun 20, 2017

rosygupta Jul 19, 2017

jreback commented Jul 19, 2017

pep8speaks commented Jul 19, 2017 •

edited

Loading

gfyoung commented Jul 21, 2017

jreback left a comment

jreback Jul 21, 2017

jreback commented Sep 23, 2017

jreback commented Oct 28, 2017

jreback commented Nov 23, 2017

		@@ -175,6 +175,14 @@ def test_to_datetime_format_weeks(self):

		class TestToDatetime(object):

		def test_to_datetime_iso_week_year_format(self):

ENH: to_datetime support iso week year (16607) #16661

ENH: to_datetime support iso week year (16607) #16661

Conversation

rosygupta commented Jun 10, 2017 • edited Loading

gfyoung commented Jun 11, 2017

rosygupta commented Jun 11, 2017

rosygupta commented Jun 11, 2017

gfyoung commented Jun 11, 2017 • edited Loading

rosygupta commented Jun 11, 2017 • edited Loading

gfyoung commented Jun 11, 2017

rosygupta commented Jun 11, 2017

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

gfyoung commented Jun 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 12, 2017

Codecov Report

codecov bot commented Jun 12, 2017 • edited Loading

Codecov Report

rosygupta commented Jun 13, 2017

gfyoung commented Jun 13, 2017 • edited Loading

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

Choose a reason for hiding this comment

jreback commented Jul 19, 2017

pep8speaks commented Jul 19, 2017 • edited Loading

Comment last updated on July 19, 2017 at 20:15 Hours UTC

gfyoung commented Jul 21, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Sep 23, 2017

jreback commented Oct 28, 2017

jreback commented Nov 23, 2017

rosygupta commented Jun 10, 2017 •

edited

Loading

gfyoung commented Jun 11, 2017 •

edited

Loading

rosygupta commented Jun 11, 2017 •

edited

Loading

codecov bot commented Jun 12, 2017 •

edited

Loading

gfyoung commented Jun 13, 2017 •

edited

Loading

pep8speaks commented Jul 19, 2017 •

edited

Loading