Repository navigation
fix: skip truncate_datetime for date and timedelta objects - #617
Sanjays2402 wants to merge 1 commit into
Conversation
DeepDiff(..., truncate_datetime=...) raised TypeError for datetime.date
values ('replace() got an unexpected keyword argument microsecond') and
AttributeError for datetime.timedelta values, because datetime_normalize
applied the truncation unconditionally to every value routed through it.
date has no time components to truncate and timedelta has no replace(),
so the truncation is now applied only to datetime and time objects.
Truncation of datetime/time values is unchanged.
Fixes qlustered#564
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #617 +/- ##
=======================================
Coverage 95.10% 95.10%
=======================================
Files 17 17
Lines 4900 4900
=======================================
Hits 4660 4660
Misses 240 240 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
seperman
left a comment
There was a problem hiding this comment.
Static review: #617 fix: skip truncate_datetime for date and timedelta objects
Summary: Makes datetime_normalize apply truncate_datetime only to datetime.datetime and datetime.time objects. This fixes the date crash reported in #564. It also fixes a remaining crash for timedelta: _diff_time sends timedelta values to datetime_normalize, which calls .replace() on them.
Overlap with other PRs
- The
datepart of #564 is already fixed ondevby #597 (merged). #597 adds ahas_timecheck athelper.py:694-696(ondev) that skips truncation only for plaindate. #602 was an earlier fix for the same issue; it was closed because the fix is already ondev. - What this PR adds beyond #597:
timedelta+truncate_datetime: ondev,has_timeisTruefortimedelta, so_diff_time(diff.py:1993-1997ondev) still callstimedelta.replace(...)and raisesAttributeError. This PR fixes that.np.datetime64undertruncate_datetimein DeepHash: it reachesdatetime_normalizethroughDeepHash._prep_datetime, since it is intimes. As far as I knownp.datetime64has no.replace(), so it should also raise ondev. With this PR it would be passed through without truncation. That avoids the crash but silently ignores the option fornp.datetime64.
- The PR targets
master, and its change athelper.py:693conflicts textually with #597's change ondev. - No overlap with #635. That PR changes
number_to_string, a different code path. Both PRs append totests/test_diff_datetime.pyand change itsdatetimeimport line, so a small rebase will be needed.
Security
No security concerns found in static review. The diff is a one-line condition change plus a test. There is no I/O and no dependency or CI change, and the diff has no non-ASCII or hidden characters.
Correctness
deepdiff/helper.py:693: the positiveisinstance(obj, (datetime.datetime, datetime.time))check is tighter thandev's negativehas_timecheck, because it coversdate,timedeltaandnp.datetime64together.datetimesubclasses (e.g. pandasTimestamp) are still truncated as before.timetruncation is unchanged.- Skipping truncation for
timedeltais consistent with the docs, which describetruncate_datetimeas applying to datetime objects. Whethertruncate_datetime='second'should also ignore sub-secondtimedeltadifferences is a separate design question. After this PR such values are compared exactly.
Tests
tests/test_diff_datetime.py:127: thedateassertions already pass ondev(since #597) and repeattest_truncate_datetime_with_dateintests/test_diff_other.py. Thetimedeltaassertion goes throughdatetime_normalizeand should fail ondevwithAttributeError, so it is the part of the test that adds coverage.- There's no equal-
timedeltacase (e.g.timedelta(days=1)vstimedelta(days=1)returns{}).
Suggestions
- Retarget to
devand rebase. When resolving the conflict athelper.py:693, replace #597'shas_timelines with this positive check. - Keep the
timedeltatest and drop the duplicateddateassertions, or move them next to #597's test. Consider annp.datetime64DeepHash case such asDeepHash(np.datetime64('2020-05-17T22:15:30'), truncate_datetime='minute')to document the chosen behaviour.
AI-assisted static review; the code was not executed.
Fixes #564
datetime_normalizeappliedtruncate_datetimeto every value routed through it, soDeepDiff(..., truncate_datetime=...)raisedTypeError: replace() got an unexpected keyword argument 'microsecond'fordatetime.datevalues andAttributeErrorfordatetime.timedeltavalues.datehas no sub-day components to truncate andtimedeltahas noreplace(), so the truncation is now applied only todatetimeandtimeobjects; truncation of those is unchanged.Regression test added in
tests/test_diff_datetime.py: it fails onmasterwith the reportedTypeErrorand passes with the fix.This change was prepared with AI assistance; the regression test was run locally and fails without the fix.