gh-64376: Use Argument Clinic for datetime.date classmethods#20208
gh-64376: Use Argument Clinic for datetime.date classmethods#20208hauntsaninja wants to merge 12 commits into
Conversation
Note that this will now throw OverflowError instead of ValueError when using the C version of datetime. There is precedent for this, e.g., datetime.date.fromordinal will throw an OverflowError with the C module but ValueError with the Python.
|
Re: |
Sorry, something went wrong.
|
Two small notes:
|
Sorry, something went wrong.
Indeed, it's good to keep these Argument Clinic conversions as straightforward as possible. |
Sorry, something went wrong.
|
Closing and re-opening to restart the hanged Travis-CI check. |
Sorry, something went wrong.
|
Thanks, looks like everything's green :-) |
Sorry, something went wrong.
taleinat
left a comment
There was a problem hiding this comment.
Thanks for this!
Needs a few tweaks but overall looks good.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
Co-authored-by: Tal Einat <532281+taleinat@users.noreply.github.com>
hauntsaninja
left a comment
There was a problem hiding this comment.
Thanks for the review! :-)
Sorry, something went wrong.
In my opinion this is fine, huge PRs can be hard to work with. |
Sorry, something went wrong.
|
To respond to all points re. types of exceptions raised:
As I stated above, I suggest keeping the behavior of raising ValueError rather than OverflowError.
Changing this to raise ValueError instead of OverflowError would be an improvement IMO.
These inconsistencies are unfortunate, but relatively minor. If this PR fixes that, adding a test for it would be great! |
Sorry, something went wrong.
https://bugs.python.org/issue20177