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

Fix *_DAYS_FROM_YEAR_0 calculation #1312

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

pitdicker
Copy link
Collaborator

@pitdicker pitdicker commented Sep 23, 2023

The MIN_DAYS_FROM_YEAR_0 and MAX_DAYS_FROM_YEAR_0 constants are only used in tests.

As described in #1048 there is something subtly wrong in the derivation of them. But I was not smart enough to put my finger on it 😄.

The formula was only correct if MIN_YEAR was a leap year. I changed it to a correct formula that tries to be less smart, and tested it to be correct for all possible years.

test_date_bounds helps to verify NaiveDate::MIN and NaiveDate::MAX are correct. I changed it a little to also report the last ordinal day in NaiveDate::MAX.

Split out from #1048.

@codecov
Copy link

codecov bot commented Sep 23, 2023

Codecov Report

Merging #1312 (913c502) into 0.4.x (a47e0e3) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##            0.4.x    #1312      +/-   ##
==========================================
- Coverage   91.24%   91.23%   -0.01%     
==========================================
  Files          38       38              
  Lines       17062    17063       +1     
==========================================
  Hits        15568    15568              
- Misses       1494     1495       +1     
Files Changed Coverage Δ
src/naive/date.rs 96.20% <0.00%> (-0.05%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pitdicker pitdicker merged commit 0953e4a into chronotope:0.4.x Sep 25, 2023
36 of 37 checks passed
@pitdicker pitdicker deleted the fix_days_from_year_0 branch September 25, 2023 08:10
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