Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0.4: Explicitly check for date while parsing so that datetime does not match #79

Merged
merged 3 commits into from
Mar 5, 2021

Conversation

dmosorast
Copy link
Contributor

@dmosorast dmosorast commented Mar 5, 2021

Description of change

While debugging a separate issue, I noticed that datetimes of the format "\/Date(1419937200000+0000)\/" as documented by Xero here were being truncated to date only.

The root of the issue was that isinstance(value, date) also matches datetime data types, so both were going down that path.

This PR makes the check explicitly for date only to avoid any conflict.

Manual QA steps

  • Ran through this in a PDB session and it worked out for both date and datetime

Testing Notes

Original behavior

ipdb> value
datetime.datetime(2020, 5, 13, 14, 20, 6, 547000)
ipdb> isinstance(value, date)
True
ipdb> isinstance(value, datetime)
True
ipdb> testdt = date(1976,10,10)
ipdb> isinstance(testdt, datetime)
False
ipdb> isinstance(testdt, date)
True

New Behavior

ipdb> testdt = date(1976,10,10)
ipdb> type(testdt) is date
True
ipdb> type(testdt) is datetime
False
ipdb> testdtt = datetime(1976,10,10,1,2,3)
ipdb> type(testdtt) is datetime
True
ipdb> type(testdtt) is date
False

Risks

  • Low, it's a relatively straightforward change, albeit a subtle cause.

Rollback steps

  • revert this branch, bump patch version, re-release

@dmosorast dmosorast merged commit 2db275c into master Mar 5, 2021
@dmosorast dmosorast deleted the fix/date-time-truncation-while-parsing branch March 5, 2021 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants