Skip to content

fix: add missing units to datetime/timedelta in a test#3688

Merged
TaiSakuma merged 2 commits intoscikit-hep:mainfrom
TaiSakuma:fix-test-2377
Oct 24, 2025
Merged

fix: add missing units to datetime/timedelta in a test#3688
TaiSakuma merged 2 commits intoscikit-hep:mainfrom
TaiSakuma:fix-test-2377

Conversation

@TaiSakuma
Copy link
Copy Markdown
Member

@TaiSakuma TaiSakuma commented Oct 20, 2025

The datetime64 and timedelta64 need a unit, e.g., datetime64[15us]
This PR adds missing units in test_2377_empty_index.py

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 20, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.71%. Comparing base (b749e49) to head (e119c0f).
⚠️ Report is 450 commits behind head on main.

Additional details and impacted files

see 200 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ianna
Copy link
Copy Markdown
Member

ianna commented Oct 21, 2025

The datetime64 and timedelta64 need a unit, e.g., datetime64[15us] This PR adds missing units in test_2377_empty_index.py

@TaiSakuma - thanks for looking into it. I think, 15us is a value, not a unit though. us is a unit.

@TaiSakuma
Copy link
Copy Markdown
Member Author

TaiSakuma commented Oct 21, 2025

@ianna - True, 15us is a value. Here it's used as a unit in the sense that 1 unit of a datetime variable with this dtype corresponds to 15 microseconds.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 23, 2025

The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3688

@ianna ianna added the pr-next-release Required for the next release label Oct 23, 2025
@TaiSakuma
Copy link
Copy Markdown
Member Author

TaiSakuma commented Oct 23, 2025

@ianna I can see the button Squash and merge now. Is it ok if I press it?

ianna
ianna approved these changes Oct 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@TaiSakuma - great! Thanks for fixing it. Please go ahead and merge it!

@TaiSakuma TaiSakuma merged commit 8097787 into scikit-hep:main Oct 24, 2025
41 checks passed
@TaiSakuma TaiSakuma deleted the fix-test-2377 branch October 24, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ianna ianna ianna approved these changes

Assignees

No one assigned

Labels

pr-next-release Required for the next release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TaiSakuma @ianna