Support dates all the way to 1BC by changhc · Pull Request #77 · pydantic/speedate (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation8 Commits7 Checks7 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This is my attempt to fix pydantic/pydantic#8570. Tested this locally with pydantic-core and it worked.

The reason of that original error is underflow. I'm not very sure what is so special about year 1600 and why that was introduced. It seems that it was once year 1000 in c7f9257 but somehow it was revert back to 1600. Considering @davidhewitt's comment #55 (review), since the minimum time string allowed in RFC 3339 is 0000-01-01T00:00:00+00:00 (UTC since other timestamp constants are also in UTC,) I think it makes sense to replace that year 1600 constant with the minimum year 0000, which will help get rid of the underflow problem.

I added some test cases for the minimum timestamp and other existing test cases still work, but I'm not sure if I missed out anything, especially because I still don't know why 1600 was used in the first place.

Note that now watershed does not cover the entire range of acceptable negative timestamp values. I changed that value to cover the entire range at first, but then I reverted the change since that would probably affect existing users relying on that value.

changhc marked this pull request as ready for review

October 25, 2024 21:10

Choose a reason for hiding this comment

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

Thanks for working on this, this seems correct to me!

I assume that this will fix #15; can we please add some additional test cases around leap years to check that this is indeed the case?

Comment on lines -151 to -154

let d = Date::from_timestamp(-11_676_096_000 + 1000, false).unwrap();
assert_eq!(d.to_string(), "1600-01-01");
let d = Date::from_timestamp(-11_673_417_600, false).unwrap();
assert_eq!(d.to_string(), "1600-02-01");

Choose a reason for hiding this comment

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

Can we restore these test cases? Not immediately clear to me why they were deleted.

Choose a reason for hiding this comment

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

I removed them because this test case is called date_from_timestamp_extremes and 1600 is no longer "an extreme value" (and I added 0000-01-01 instead.) I'll create a new test case and add them back.

I've added 3 cases checking 1 BC, 1200 AD and 1600 AD. Is there anything else specific I should add? I think other cases are already covered.

It would be good to check February 29th of some leap years in that 0-1599 range to confirm we are handling them correctly.

From wikipedia

Every year that is exactly divisible by four is a leap year, except for years that are exactly divisible by 100, but these centurial years are leap years if they are exactly divisible by 400. For example, the years 1700, 1800, and 1900 are not leap years, but the year 2000 is.

So I guess February 29th should be valid on 1200, but not 1300, 1400, 1500.

Similarly we could check 1200 (yes), 1201 (no), 1202 (no), 1203 (yes).

I see. I've added some more test cases. Definitely not covering the entire range 0-1600 but I think these should be enough, unless we really want more of these tests.

Choose a reason for hiding this comment

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

Perfect, this looks great to me! Thanks 🙏

davidhewitt changed the title~~Replace UNIX 1600 with UNIX 0000~~ Support dates all the way to 1BC

Nov 1, 2024

renovate bot added a commit to spiraldb/ziggy-pydust that referenced this pull request

Nov 20, 2024

renovate bot added a commit to MartinBernstorff/Memium that referenced this pull request

Nov 20, 2024

kodiakhq bot added a commit to cloudquery/python-plugin-template that referenced this pull request

Dec 9, 2024

This PR contains the following updates:

Package	Type	Update	Change	Pending
pydantic (changelog)	dependencies	minor	`2.4.2` -> `2.10.2`	`2.10.3`

Release Notes

pydantic/pydantic (pydantic)

Support dates all the way to 1BC by changhc · Pull Request #77 · pydantic/speedate (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Release Notes

What's Changed

Fixes

What's Changed

Packaging

Fixes

New Contributors

What's Changed

Packaging

New Features

Changes

Performance

Fixes

New Contributors

What's Changed

Fixes

What's Changed

Fixes

What's Changed

Packaging

New Features

Changes

Performance

Minor Internal Improvements

Fixes

New Contributors

pydantic

pydantic-core

What's Changed

Fixes

What's Changed

Packaging

Fixes

What's Changed

Packaging

New Features

Changes

Performance

Internal Improvements

Fixes

Configuration

Release Notes

What's Changed

Fixes

What's Changed

Fixes

What's Changed

Packaging

Fixes

New Contributors

What's Changed

Fixes

What's Changed

Fixes

What's Changed

Packaging

Fixes

New Contributors

What's Changed

Packaging

New Features

Changes

Performance

Fixes

New Contributors

What's Changed

Fixes

What's Changed

Fixes

What's Changed

Packaging

New Features

Changes

Performance

`pydantic`

`pydantic-core`

`pydantic`