BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor #26848

jorisvandenbossche · 2019-06-14T10:14:08Z

closes REGR? no error anymore when converting out of bounds datetime64[non-ns] data #26206
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

TODO: Need to add a whatsnew and test for DataFrame.

Question:

I am now letting this raise an error. But, an alternative could be to return an object array (by simply removing the except OutOfBoundsDatetime: raise, as the code below that fall backs to object array if no dtype is specified.

This would be more consistent with if it is not an array but list of scalars:

In [5]: a = np.array(['2262-04-12'], dtype='datetime64[s]')

In [6]: pd.Series(a)   # now raising on this PR
...
OutOfBoundsDatetime: Out of bounds nanosecond timestamp: 2262-04-12 00:00:00

In [7]: pd.Series([a[0]])    # already falling back to object array on master
Out[7]: 
0    2262-04-12T00:00:00
dtype: object

That is somewhat the general logic of the _try_cast. Of course, if you specify the required dtype explicitly, you should still get an error (which is actually also a bug on master, I see now).

However, that would be inconsistent with the current pd.Index behaviour, which raises when passing out of bounds datetime64 (but also returns object dtype with list of datetime64 scalars)

cc @jbrockmendel @mroeschke

pandas/core/internals/construction.py

codecov · 2019-06-14T10:52:02Z

Codecov Report

Merging #26848 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26848      +/-   ##
==========================================
- Coverage   91.97%   91.97%   -0.01%     
==========================================
  Files         180      180              
  Lines       50756    50760       +4     
==========================================
+ Hits        46685    46686       +1     
- Misses       4071     4074       +3

Flag	Coverage Δ
#multiple	`90.56% <100%> (ø)`	⬆️
#single	`41.84% <33.33%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/dtypes/cast.py	`90.56% <100%> (+0.03%)`	⬆️
pandas/core/internals/construction.py	`95.95% <100%> (+0.02%)`	⬆️
pandas/io/gbq.py	`88.88% <0%> (-11.12%)`	⬇️
pandas/core/frame.py	`96.89% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.94% <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 b9b081d...369eead. Read the comment docs.

codecov · 2019-06-14T10:52:02Z

Codecov Report

Merging #26848 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26848      +/-   ##
==========================================
- Coverage   91.86%   91.85%   -0.01%     
==========================================
  Files         179      179              
  Lines       50700    50703       +3     
==========================================
- Hits        46576    46575       -1     
- Misses       4124     4128       +4

Flag	Coverage Δ
#multiple	`90.45% <100%> (ø)`	⬆️
#single	`41.12% <40%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/dtypes/cast.py	`90.53% <100%> (ø)`	⬆️
pandas/core/internals/construction.py	`95.96% <100%> (+0.03%)`	⬆️
pandas/io/gbq.py	`88.88% <0%> (-11.12%)`	⬇️
pandas/core/frame.py	`96.88% <0%> (-0.12%)`	⬇️

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 8374a1d...c5fc3c6. Read the comment docs.

jorisvandenbossche · 2019-06-14T11:12:28Z

not sure how this is inconsistent

Because we have been doing the opposite for some releases?

Before 0.23.0, all of Index, Series and DataFrame constructors have been consistently raising an error when out-of-bounds datetime64 array was passed, even if no explicit dtype was specified.

So yes, I consider converting to object an option (this is why I raised the question in the top post), but it's not necessarily the "only correct" way. For example, for Index, this would be a change in behaviour.

jreback · 2019-06-14T11:20:41Z

Before 0.23.0, all of Index, Series and DataFrame constructors have been consistently raising an error when out-of-bounds datetime64 array was passed, even if no explicit dtype was specified.

this is likely a bug / regression

Series constructor was designed to not raise and rather coerce

jorisvandenbossche · 2019-06-14T11:22:20Z

Series constructor was designed to not raise and rather coerce

I suppose you don't mean "coerce" here in meaning of to_datetime, as that is returning NaT ?

jorisvandenbossche · 2019-06-14T15:38:51Z

@jreback looked at some different cases and made a general issue here: #26853

mroeschke · 2019-06-17T06:25:27Z

pandas/tests/series/test_constructors.py

+    def test_constructor_datetime64_outofbound(self):
+        # GH-26206 out of bound non-ns unit
+        with pytest.raises(pd.errors.OutOfBoundsDatetime):
+            pd.Series(np.array(['2262-04-12'], dtype='datetime64[D]'))


Could you also test the minimum date as well?

It's relying on a general routine that does this bound checking and which is already tested thoroughly through other means, here I mainly want to test to ensure this routine is used. Therefore, I decided to keep the tests here a bit simpler (adding is not that hard, but it just complicates the test further, and there are already several cases). Is that OK?

…datetime-unit-out-of-bounds

pep8speaks · 2019-06-17T08:49:58Z

Hello @jorisvandenbossche! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-06-21 06:16:22 UTC

pandas/core/dtypes/cast.py

jreback

code change looks good (ideally we want to simplify that code, but that is non-trivial / out-of scope for this). can you add a whatsnew note (I think single line is ok, not sure we need a sub-section to explain, though ok for that too)

…datetime-unit-out-of-bounds

jorisvandenbossche · 2019-06-18T11:28:08Z

One question about the tests: I want to add the exact same for DataFrame and Index constructors. I can copy an edited version to the respective testing files for DataFrame and Index, but on the other hand, I think there is also benefit in keeping the test together (less duplication of the boiler plate around it, could do a parametrize). Do we have a place to put general constructor tests that check for consistency? (because doing it here in the series tests is also a bit out of place)

jreback · 2019-06-18T11:30:37Z

One question about the tests: I want to add the exact same for DataFrame and Index constructors. I can copy an edited version to the respective testing files for DataFrame and Index, but on the other hand, I think there is also benefit in keeping the test together (less duplication of the boiler plate around it). Do we have a place to put general constructor tests that check for consistency? (because doing it here in the series tests is also a bit out of place)

we have 2 mechanisms for this, neither complete solutions. test_generic you can do Series/DataFrame and test_base you can do Series/Index.

generally for this as its really the Series you are fixing is to put it in test_base and then copy a similar example to the appropriate palace in frame tests.

jorisvandenbossche · 2019-06-18T11:37:59Z

My last commit does an example parametrize of Index/DataFrame/Series

generally for this as its really the Series you are fixing

yes, but that also fixes DataFrame (with passing a dict) as it is using the same code. For Index it is indeed only adding tests to ensure consistency.

jorisvandenbossche · 2019-06-18T11:49:54Z

Will move to test_base, that seems most appropriate (it indeed already has some tests that checks consistency across Series, Index)

jorisvandenbossche · 2019-06-18T12:00:05Z

What both the dataframe and series fix have in common is that the code (where the fixes are applied) is now centralized in construction.py. So it might make sense to also have a test_construction.py at some point.

jorisvandenbossche · 2019-06-18T12:16:16Z

This should be ready. I opened a new issue for the xfail DataFrame case that I added, as this is a more general issue that we are doing unsafe astype: #26919

doc/source/whatsnew/v0.25.0.rst

jreback · 2019-06-21T02:19:25Z

pls merge master. lgtm. merge on green.

…unds

BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor

c5fc3c6

jorisvandenbossche added Regression Functionality that used to work in a prior pandas version Datetime Datetime data dtype labels Jun 14, 2019

jorisvandenbossche added this to the 0.25.0 milestone Jun 14, 2019

jreback requested changes Jun 14, 2019

View reviewed changes

pandas/core/internals/construction.py Show resolved Hide resolved

jorisvandenbossche mentioned this pull request Jun 14, 2019

API: handling of invalid datetimes in constructors #26853

Open

mroeschke reviewed Jun 17, 2019

View reviewed changes

jorisvandenbossche added 3 commits June 17, 2019 10:34

Update tests according to discussion

a0c7051

update fix

b69f82e

Merge remote-tracking branch 'upstream/master' into pandas-devGH-26206-…

3d50970

…datetime-unit-out-of-bounds

lint

4d28cad

jreback requested changes Jun 17, 2019

View reviewed changes

pandas/core/dtypes/cast.py Show resolved Hide resolved

jreback requested changes Jun 17, 2019

View reviewed changes

jorisvandenbossche added 2 commits June 18, 2019 13:17

Merge remote-tracking branch 'upstream/master' into pandas-devGH-26206-…

67c9e7c

…datetime-unit-out-of-bounds

add whatsnew

2e431b6

parametrize example

ef89acf

jorisvandenbossche added 2 commits June 18, 2019 13:40

xfail dataframe from array case

bbb59b1

move tests

bf2e77c

jorisvandenbossche mentioned this pull request Jun 18, 2019

BUG: DataFrame(ndarray, dtype=..) does unsafe conversion #26919

Closed

lint

fd742b7

jreback reviewed Jun 18, 2019

View reviewed changes

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved

update whatsnew

968d854

jorisvandenbossche requested a review from jreback June 19, 2019 12:30

jreback approved these changes Jun 21, 2019

View reviewed changes

Merge branch 'master' into pandas-devGH-26206-datetime-unit-out-of-bo…

369eead

…unds

jorisvandenbossche merged commit c275dbf into pandas-dev:master Jun 21, 2019

jorisvandenbossche deleted the GH-26206-datetime-unit-out-of-bounds branch June 21, 2019 07:05

jorisvandenbossche mentioned this pull request Jun 21, 2019

CLN: clean-up sanitize_array series construction #26979

Merged

TomAugspurger mentioned this pull request Dec 30, 2019

REGR? no error anymore when converting out of bounds datetime64[non-ns] data #26206

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor #26848

BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor #26848

jorisvandenbossche commented Jun 14, 2019

codecov bot commented Jun 14, 2019 •

edited

Loading

codecov bot commented Jun 14, 2019

jorisvandenbossche commented Jun 14, 2019

jreback commented Jun 14, 2019

jorisvandenbossche commented Jun 14, 2019 •

edited

Loading

jorisvandenbossche commented Jun 14, 2019

mroeschke Jun 17, 2019

jorisvandenbossche Jun 18, 2019

pep8speaks commented Jun 17, 2019 •

edited

Loading

jreback left a comment

jorisvandenbossche commented Jun 18, 2019 •

edited

Loading

jreback commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jreback commented Jun 21, 2019

BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor #26848

BUG: catch out-of-bounds datetime64 in Series/DataFrame constructor #26848

Conversation

jorisvandenbossche commented Jun 14, 2019

codecov bot commented Jun 14, 2019 • edited Loading

Codecov Report

codecov bot commented Jun 14, 2019

Codecov Report

jorisvandenbossche commented Jun 14, 2019

jreback commented Jun 14, 2019

jorisvandenbossche commented Jun 14, 2019 • edited Loading

jorisvandenbossche commented Jun 14, 2019

mroeschke Jun 17, 2019

Choose a reason for hiding this comment

jorisvandenbossche Jun 18, 2019

Choose a reason for hiding this comment

pep8speaks commented Jun 17, 2019 • edited Loading

Comment last updated at 2019-06-21 06:16:22 UTC

jreback left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Jun 18, 2019 • edited Loading

jreback commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jorisvandenbossche commented Jun 18, 2019

jreback commented Jun 21, 2019

codecov bot commented Jun 14, 2019 •

edited

Loading

jorisvandenbossche commented Jun 14, 2019 •

edited

Loading

pep8speaks commented Jun 17, 2019 •

edited

Loading

jorisvandenbossche commented Jun 18, 2019 •

edited

Loading