ENH: ISO8601-compliant datetime string conversion in `iterrows()` and Series construction.
State: closed

- [x] closes https://github.com/pandas-dev/pandas/issues/19671
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Datetime
description: Datetime data dtype
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Hello @minggli! Thanks for updating the PR.









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

##### Comment last updated on February 25, 2018 at 23:05 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=h1) Report
> Merging [#19762](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/8f1dfa74e244622a7f356a496072f93dce032839?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/19762/graphs/tree.svg?width=650&height=150&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #19762      +/-   ##
==========================================
+ Coverage   91.67%   91.69%   +0.02%     
==========================================
  Files         150      150              
  Lines       48936    48938       +2     
==========================================
+ Hits        44860    44872      +12     
+ Misses       4076     4066      -10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.06% <100%> (+0.02%)` | :arrow_up: |
| #single | `41.81% <83.33%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/internals.py](https://codecov.io/gh/pandas-dev/pandas/pull/19762/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzLnB5) | `95.53% <ø> (ø)` | :arrow_up: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/19762/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `87.68% <100%> (-0.3%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/19762/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `66.95% <0%> (+1.73%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=footer). Last update [8f1dfa7...08d2718](https://codecov.io/gh/pandas-dev/pandas/pull/19762?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback changes implemented and added test and whatsnew doc.
General question: is this important enough to add this to the public API?  (the user can already specify the format, although that might not be as flexible as such a keyword) 
Because if we need it internally for eg `iterrows`, we can keep the keyword internal to solve that specific problem without exposing it in the public `to_datetime` ?

Further, it does not seem to fully work:

```
In [15]: pd.to_datetime("2018/02/22 12:14:23", require_iso8601=True)
Out[15]: Timestamp('2018-02-22 12:14:23')
```

The above is not a ISO8601 format, so should not be allowed. If we make the keyword public, the name should reflect what it does correctly (for internal usage the above might be OK?)
from @jorisvandenbossche comments: https://github.com/pandas-dev/pandas/pull/19762#issuecomment-367690314

ok to add this as a private variable instead, we don't really have a convention for this, maybe use
``_require_iso8601=False`` as the arg
from @jorisvandenbossche comments: #19762 (comment)

the documentation suggests it's (almost) standard ISO8601 with small differences, so seems not meant to be exact.

https://github.com/pandas-dev/pandas/blob/abc4ef9132d8bcc8ad987c18fdd757595fd65d8c/pandas/_libs/src/datetime/np_datetime_strings.h#L26

I think as a product manager previously, it'd be better to expose it in public so user has choice of parsing behaviour as long as doc is up-to-date, as opposed to having no choice. If we want to keep it internal, it would still show up in doc (e.g. Series(fastpath=)) but without documentation as parameter or examples.
we actually have a failrly flexibile version of ISO8601, meaning that we accept a bunch of separators that are not in the standard (and whitespace). that's why https://github.com/pandas-dev/pandas/pull/19762#issuecomment-367690314 is parseable.

So I think on balance its better not to add this to the public API. @minggli can you change to using ``_require_iso8601`` (IOW a private keyword).?
all the tests are fine. can you remove the example from the doc-string, but leave the wiki reference (move to the Notes section).
no problem, I will make it `_require_iso8601`. two questions: where is Notes section? do you mean parameter?; would it be better to use `**kwargs` so that we don't show this keyword at all and also prevent any breaking changes?
Can we prevent all together to add a keyword to `to_datetime`? As adding a 'private' keyword to a public function is not that nice.
I am not completely familiar with all our different datetime parsing functions, but it seems to me that the `require_iso8601` flag is *only* used in `to_datetime` if the code path ends up in `tslib.array_to_datetime`. And the only place internally in pandas where we need this flag is in `maybe_infer_to_datetimelike`. Can't we try to fix it directly there?
So in `to_datetime`, if `tslib.array_to_datetime` has failed, it tries `conversion.datetime_to_datetime64(arg)` which converts all-datetime-like (datetime.datetime, Timestamp) object array. I think that is exactly what we want in `maybe_infer_to_datetimelike`, so I think we can use that directly.
I am not sure what you mean @jorisvandenbossche . If we want to hard code require_iso8601=True, it will solve the issue in question, but it will also change to_datetime behaviour.

to_datetime(['M1809', 'M1701', Timestamp('20130101')], errors='raise') won't raise ValueError before, but if we do hard code the arg internally in order to remove the public kwarg, it will raise ValueError. As per @jreback 's comment in https://github.com/pandas-dev/pandas/issues/19671 , datetime string like 'M1701' although not ISO8601, is legit, so to_datetime should convert it without raising an error. 
>  but it will also change to_datetime behaviour.

My proposal is to *not* change `to_datetime`, but to only make changes to `maybe_infer_to_datetimelike`, to solve the actual problem (that we internally convert to aggressively to datetimes, eg in iterrows). As it is that function that needs the `require_iso8601` behaviour.
yes `maybe_infer_to_datetimelike` is definitely one to look at. the actual conversion happens in `try_datetime` which tries array_to_datetime and then to_datetime to handle some exceptions. I think to_datetime is called for a reason and change `try_datetime` may end up breaking old logics. 

To fix the issue at hand, require_iso8601=True is needed for both calls, either False won't work. but  `to_datetime` defaults to False.

Therefore the best way I can approach what you are thinking about is https://github.com/pandas-dev/pandas/pull/19762/commits/f9240b5285f5abf611ec31d4aa5827c0f17bb58d which fix the issue (require_iso8601=True where it needs to be without breaking other things) and doesn't change to_datetime behaviour (still default require_iso8601=False) or expose the keyword to public. 
Can you try to change the `to_datetime` call in `maybe_infer_to_datetimelike` with `conversion.datetime_to_datetime64(arg)`? (that's what I meant in https://github.com/pandas-dev/pandas/pull/19762#issuecomment-368011627) And check if that solves the issue as well?
I changed the construction logic slightly. can you also add the original test (e.g .the iterrows) example?
@jreback done.
@jreback SparseDataFrame.values seem to have lost dtype along the way, breaking test_iterrows(). Dense dataframe (s.to_dense().values) doesn't have this problem. is it normal? 

>>> s = SparseDataFrame(
...     {'non_iso8601': ['M1701', 'M1802', 'M1903'],
...      'iso8601': to_datetime(['2016-09-01', '2017-01-01', '2018-01-23'])})
>>>
>>> s.values
array([[1472688000000000000, 'M1701'],
       [1483228800000000000, 'M1802'],
       [1516665600000000000, 'M1903']], dtype=object)
>>>
>>> s.to_dense().values
array([[Timestamp('2016-09-01 00:00:00'), 'M1701'],
       [Timestamp('2017-01-01 00:00:00'), 'M1802'],
       [Timestamp('2018-01-23 00:00:00'), 'M1903']], dtype=object)

s._data.as_array()
array([[1472688000000000000, 1483228800000000000, 1516665600000000000],
       ['M1701', 'M1802', 'M1903']], dtype=object)
looked into above, but SparseFrame doesn't have the granularity that dense DataFrame has in terms of handling different block types.

DatetimeLikeBlockMixin provides Timestamp, Timedelta constructions in Datetimelike Blocks. 

This is a pre-existing condition which I think is outside the scope of this PR. moved test case for DataFrame only.
@minggli code/tests lgtm. I just pushed a tiny update. Can you add a whatsnew note (bug fix, reshaping). ping on green.
Hi @jreback, thanks for the update. I initially put test in SharedwithSparse like you just did and it fails on tests.sparse.frame due to reasons mentioned above.
> Hi @jreback, thanks for the update. I initially put test in SharedwithSparse like you just did and it fails on tests.sparse.frame due to reasons mentioned above.

ahh ok. can you put an xfail on it then for the Sparse (I *think* there are some examples like that)
thanks @minggli very nice! keep em coming!
happy to help!!