fix timedelta to datetime conversion #577

d-chambers · 2025-10-23T15:47:30Z

Description

This PR allows the conversion of timedelta64 to datetime64 arrays. Currently it raises a TypeError.

Checklist

I have (if applicable):

referenced the GitHub issue this PR closes.
documented the new feature with docstrings and/or appropriate doc page.
included tests. See testing guidelines.
added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

Bug Fixes
- Enhanced numeric data handling in datetime conversions for improved type safety and edge-case processing.
Tests
- Added test coverage for timedelta64 to datetime64 conversion verification.

coderabbitai · 2025-10-23T15:47:40Z

Walkthrough

Modified _array_to_datetime64 to explicitly convert numerical data to float using to_float(np.array(array)) for proper NaN handling. Updated tests to use existing random_state fixture instead of creating new instances. Added test for timedelta64 array conversion.

Changes

Cohort / File(s)	Change Summary
Time utilities implementation `dascore/utils/time.py`	Modified `_array_to_datetime64` numerical data branch to wrap input in `to_float(np.array(array))` for explicit float conversion before NaN handling and downstream arithmetic operations.
Time utilities tests `tests/test_utils/test_time.py`	Replaced RandomState instantiation with existing random_state fixture in two tests. Added `TestToDateTime64.test_timedelta_array` test to verify timedelta64 to datetime64 dtype preservation.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "fix timedelta to datetime conversion" is concise, clear, and directly summarizes the main change in the pull request. According to the raw summary and PR objectives, the primary goal is to fix the conversion of timedelta64 arrays to datetime64 arrays, which was previously raising a TypeError. The title accurately reflects this core change without vague terminology, noise, or unnecessary details. The title may not describe every detail of the implementation (such as the float conversion wrapping), but it adequately captures the high-level objective that a teammate would understand when scanning history.
Description Check	✅ Passed	The PR description follows the required template structure with both a Description section and a Checklist section present and correctly formatted. The description section clearly explains what the PR accomplishes—enabling conversion of timedelta64 to datetime64 arrays, which currently raises a TypeError—providing adequate context for the change. All required checklist items from the template are included. However, all checklist items are unchecked (marked with empty boxes), indicating the author has not confirmed completing the recommended steps such as referencing a GitHub issue, documenting the feature, or adding tests. While the raw summary shows tests were actually added, the checklist does not reflect this completion.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch datetime64_from_timedelta64

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9073eac and c9a4adf.

📒 Files selected for processing (2)

dascore/utils/time.py (1 hunks)
tests/test_utils/test_time.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/test_utils/test_time.py (1)

dascore/utils/time.py (2)

to_datetime64 (22-55)

to_timedelta64 (141-173)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)

GitHub Check: test_code (windows-latest, 3.12)
GitHub Check: test_code (windows-latest, 3.13)
GitHub Check: test_code (windows-latest, 3.11)
GitHub Check: test_code (macos-latest, 3.13)
GitHub Check: test_code (windows-latest, 3.10)
GitHub Check: test_code (ubuntu-latest, 3.11)
GitHub Check: test_code (ubuntu-latest, 3.10)
GitHub Check: test_code (macos-latest, 3.10)
GitHub Check: test_code (macos-latest, 3.11)
GitHub Check: test_code (ubuntu-latest, 3.13)
GitHub Check: test_code (ubuntu-latest, 3.12)
GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)
GitHub Check: test_code_min_deps (windows-latest, 3.12)
GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
GitHub Check: test_code_min_deps (windows-latest, 3.13)
GitHub Check: test_code_min_deps (macos-latest, 3.13)
GitHub Check: test_code_min_deps (macos-latest, 3.12)
GitHub Check: Run benchmarks

🔇 Additional comments (4)

tests/test_utils/test_time.py (3)

179-180: LGTM: Good practice using existing fixture.

Using the existing random_state fixture instead of creating new instances ensures consistency across tests and follows testing best practices.

188-192: LGTM: Good test coverage for the new feature.

The test appropriately verifies that timedelta64 arrays can be converted to datetime64 arrays and that the resulting dtype is correct.

325-326: LGTM: Consistent with the earlier fixture usage improvement.

Consistent with the change at lines 179-180, using the existing random_state fixture is the right approach.
dascore/utils/time.py (1)
98-98: Verify that timedelta64 actually reaches the code branch at line 96.

The review comment assumes np.isreal(timedelta64_scalar) returns True so timedelta64 arrays enter the branch at line 96 where to_float() is called. However, np.isreal() typically returns False for specialized numpy types like timedelta64, which would prevent this branch from being entered entirely. This would leave out undefined at line 109 (since neither the if at line 90 nor the elif at line 96 would execute).

The test passing suggests either: (1) np.isreal() behaves unexpectedly for timedelta64 in your environment, (2) timedelta64 is converted to a numeric type before reaching this function, or (3) timedelta64 never actually flows through this path. Confirm the assumption manually by testing:
import numpy as np
# Verify np.isreal() behavior with timedelta64 scalars
td = np.timedelta64(1, 's')
print(np.isreal(td))  # Likely False

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-10-23T15:50:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.92%. Comparing base (9073eac) to head (c9a4adf).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #577   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files         128      128           
  Lines       10880    10880           
=======================================
  Hits        10872    10872           
  Misses          8        8

Flag	Coverage Δ
unittests	`99.92% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codspeed-hq · 2025-10-23T16:17:02Z

CodSpeed Performance Report

Merging #577 will not alter performance

_{Comparing datetime64_from_timedelta64 (c9a4adf) with master (9073eac)}

Summary

✅ 54 untouched

fix timedelta issue

c9a4adf

d-chambers merged commit 7f7676f into master Oct 24, 2025
23 checks passed

d-chambers deleted the datetime64_from_timedelta64 branch October 24, 2025 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix timedelta to datetime conversion #577

fix timedelta to datetime conversion #577

Uh oh!

d-chambers commented Oct 23, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Oct 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix timedelta to datetime conversion #577

fix timedelta to datetime conversion #577

Uh oh!

Conversation

d-chambers commented Oct 23, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented Oct 23, 2025

CodSpeed Performance Report

Merging #577 will not alter performance

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

d-chambers commented Oct 23, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 23, 2025 •

edited

Loading

codecov bot commented Oct 23, 2025 •

edited

Loading