Skip to content

New API for date-like types unit/format configuration #11504

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sydney-runkle
Copy link
Contributor

Fixes #10454
Fixes #7940

Previously outlined this API in #10949, but continuing here with a clean history.

Copy link

cloudflare-workers-and-pages bot commented Feb 27, 2025

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: cd7d95c
Status:🚫  Build failed.

View logs

@sydney-runkle
Copy link
Contributor Author

sydney-runkle commented Feb 27, 2025

@ollz272, would love your feedback on this API proposal! Want to solidify things before I implement logic in pydantic-core.

Copy link

codspeed-hq bot commented Feb 27, 2025

CodSpeed Performance Report

Merging #11504 will not alter performance

Comparing datetime-config-settings (cd7d95c) with main (bff7477)

Summary

✅ 46 untouched benchmarks

Copy link
Contributor

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

These settings seem simple and powerful to me 👍

@ollz272
Copy link
Contributor

ollz272 commented Feb 27, 2025

@ollz272, would love your feedback on this API proposal! Want to solidify things before I implement logic in pydantic-core.

Hi!

Take with a pinch of salt as I only have access to my phone at the moment! However looking at this, all seems sensible and will achieve what we’re looking for!

really excited to see this in 2.11!

@ollz272
Copy link
Contributor

ollz272 commented Feb 28, 2025

I'm about to go on leave for a couple of weeks so if any other input from me is wanted i won't be around, @benjsec may be able to give some input my absence. Really looking forward to seeing this!

@sydney-runkle
Copy link
Contributor Author

After much discussion with the team, we've settled on the above, which closely matches some of the work that @ollz272 did previously, so I'll port that over in pydnatic-core shortly.

@sydney-runkle
Copy link
Contributor Author

@ollz272,

Unfortunate update here - I'll be scaling back my work on pydantic starting next week, so this might not get finished for the v2.11 release. That being said, I confirmed with our team internally that this is the API we want to move forward with for these requested features. If anyone wants to pick this up, we'd happily accept PRs advancing this feature.

Sorry for the churn on this feature request - it definitely fell to the bottom of our todo list a few times as features like improving the alias configuration API took priority due to popular demand. I'll leave this PR open for now (maybe someone will have the capacity to pick this up in the next month or so). @Viicos or @davidhewitt, feel free to close for cleanup purposes as you see fit (it's linked to the relevant issues, so should be easy enough to dig up later).

@ollz272
Copy link
Contributor

ollz272 commented Mar 27, 2025

Hi @sydney-runkle,

slowly coming back now. Unfortunate that this didn’t make it into 2.11, but understandable. Will this be prioritised internally for 2.12? Or will you be relying on community PR’s to push this along?

@ollz272
Copy link
Contributor

ollz272 commented Apr 1, 2025

@davidhewitt @Viicos Hi! I saw on linkedin that sydney has moved on from pydantic so maybe she wasn't the best person to ask the above question to, sorry if you guys aren't either!

Same question: Will this be prioritised internally for 2.12? Or will you be relying on community PR’s to push this along?

@Viicos
Copy link
Member

Viicos commented Apr 1, 2025

Hi @ollz272 sorry for the slow response. We'll tackle this in 2.12. It might be that we recommend using validators instead (unless not possible or incurs performance issues, I'll need to take a deeper look), as we want to avoid introducing many configuration values that affect validation behavior as this can get quickly out of hand.

@ollz272
Copy link
Contributor

ollz272 commented Apr 1, 2025

Hi @ollz272 sorry for the slow response. We'll tackle this in 2.12. It might be that we recommend using validators instead (unless not possible or incurs performance issues, I'll need to take a deeper look), as we want to avoid introducing many configuration values that affect validation behavior as this can get quickly out of hand.

Thanks! I think the main thing i was concerned with in the original issue was serialisation, so im not sure a validator would fix that. Open to other suggestions on how this could work, ofc, but really we want to avoid things like this:

T_SerializedDatetime = Annotated[
    dt.datetime,
    PlainSerializer(
        lambda x: x.timestamp() * 1000,
        return_type=int,
        when_used="json",
    ),
]

class Value(BaseModel):
    series: dict[T_SerializedDatetime, float]

as the cost of computing the x.timestamp() * 1000 many times (i guess some overhead from going back and forth from python to rust as well?) eats into a high amount of time for us.

@ollz272
Copy link
Contributor

ollz272 commented May 29, 2025

Hi @DouweM, i think from looking around you’re now managing pydantic! Congrats!

I was just hoping to get some insight into progress on this issue/pr and if we can expect it in 2.12?

@DouweM
Copy link
Contributor

DouweM commented May 29, 2025

@ollz272 Hi Oliver, I'm primarily on PydanticAI and @Viicos is the primary developer on Pydantic, so I'd need to check with him to see if his comment from April 1 about this going into 2.12 is still accurate. He's out right now and will be back next Wednesday, I'll make a note to ask him then!

@Viicos
Copy link
Member

Viicos commented May 29, 2025

Hi @ollz272 , yes this will definitely be tackled for 2.12. I'm not fully working on Pydantic these days but I'll get back around it before the release for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-fix Used for bugfixes.
Projects
None yet
5 participants