Skip to content

Fix parsing corner case closes #19382#19529

Merged
jreback merged 3 commits intopandas-dev:masterfrom
jbrockmendel:corner_parse
Feb 6, 2018
Merged

Fix parsing corner case closes #19382#19529
jreback merged 3 commits intopandas-dev:masterfrom
jbrockmendel:corner_parse

Conversation

@jbrockmendel
Copy link
Copy Markdown
Member

@jbrockmendel jbrockmendel commented Feb 4, 2018

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 4, 2018 •
edited
Loading

Codecov Report

Merging #19529 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #19529   +/-   ##
=======================================
  Coverage   91.62%   91.62%           
=======================================
  Files         150      150           
  Lines       48714    48714           
=======================================
  Hits        44633    44633           
  Misses       4081     4081
Flag Coverage Δ
#multiple 89.99% <ø> (ø) ⬆️
#single 41.75% <ø> (ø) ⬆️

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 bc1d027...0af64f6. Read the comment docs.

jreback
jreback requested changes Feb 4, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls always always add a whatsnew note for any user facing change.

The conversion code for datetimes is still all over the place because tslib.pyx still exists, would be happy to remove that.

pandas/_libs/tslib.pyx Outdated
return oresult


cdef bint _handle_error_require_iso8601(object val, int64_t* iresult,
Copy link
Copy Markdown
Contributor

@jreback jreback Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather do this inline, you are mutating the result here, and it makes the logic much harder to follow. If you want to do this in a separate PR that just refactors might be ok, but not on this one.

Copy link
Copy Markdown
Member Author

@jbrockmendel jbrockmendel Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

pandas/tests/scalar/test_timestamp.py Outdated

from pandas.tseries import offsets

from pandas._libs.tslib import OutOfBoundsDatetime
Copy link
Copy Markdown
Contributor

@jreback jreback Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from pandas.errors import

pandas/tests/indexes/datetimes/test_tools.py
@@ -1596,6 +1595,19 @@ def test_coerce_of_invalid_datetimes(self):
)
)

Copy link
Copy Markdown
Contributor

@jreback jreback Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import from pandas.errors

@jreback jreback added Bug Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas and removed Bug labels Feb 4, 2018
@jbrockmendel
Copy link
Copy Markdown
Member Author

jbrockmendel commented Feb 4, 2018

The conversion code for datetimes is still all over the place because tslib.pyx still exists, would be happy to remove that.

Yah, I've been holding off on that because there are small differences between array_to_datetime and the Timestamp constructor that I'm hoping to un-difference, then share the implementation.

@jreback jreback added this to the 0.23.0 milestone Feb 6, 2018
@jreback jreback merged commit 84522a0 into pandas-dev:master Feb 6, 2018
@jreback
Copy link
Copy Markdown
Contributor

jreback commented Feb 6, 2018

thanks. happy to have a cleanup in array_to_datetime, but that's a separate PR

@jbrockmendel jbrockmendel deleted the corner_parse branch February 11, 2018 21:39
harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018
@jbrockmendel
Fix parsing corner case closes pandas-dev#19382 (pandas-dev#19529)
9985077
@varadgunjal varadgunjal mentioned this pull request Nov 28, 2018
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jreback jreback jreback approved these changes

Assignees

No one assigned

Labels

Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas

Projects

None yet

Milestone

0.23.0

Development

Successfully merging this pull request may close these issues.

BUG: Out of bounds Timestamp does not raise exception

2 participants

@jbrockmendel @jreback