Skip to content

gh-64376: Use Argument Clinic for datetime.date classmethods #20208

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.

Sign up for GitHub

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

Closed
hauntsaninja wants to merge 12 commits into from
Closed

gh-64376: Use Argument Clinic for datetime.date classmethods #20208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ee232ca
datetime.date.fromordinal
May 19, 2020
78e71d0
datetime.date.fromisoformat
May 19, 2020
d0eac8d
datetime.date.fromisocalendar
May 19, 2020
5d9b7de
datetime.today
May 19, 2020
0497383
📜🤖 Added by blurb_it.
blurb-it[bot] May 19, 2020
13ad7c7
Merge remote-tracking branch 'upstream/master' into datetime
Jun 4, 2020
c1a2b1e
re-run argument clinic
Jun 4, 2020
e6f7572
Update Modules/_datetimemodule.c
hauntsaninja Jan 21, 2022
11f83fd
Remove NEWS entry
Jan 21, 2022
188130b
Fix datetime.date.today docstring
Jan 21, 2022
0528066
Re-run clinic
Jan 21, 2022
d6c8759
Use dtstr instead of date_string
Jan 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
datetime.date.fromisocalendar 
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.
  • Loading branch information
hauntsaninja
hauntsaninja committed May 19, 2020
commit d0eac8d99ae7c395e0c9db8565186a1ffc5ac610
3 changes: 0 additions & 3 deletions Lib/test/datetimetester.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1909,9 +1909,6 @@ def test_fromisocalendar_value_errors(self):
(10000, 1, 1),
(0, 1, 1),
(9999999, 1, 1),
(2<<32, 1, 1),
(2019, 2<<32, 1),
(2019, 1, 2<<32),
Comment on lines -1912 to -1914

@taleinat taleinat Jan 20, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are these removed?

@hauntsaninja hauntsaninja Jan 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I mentioned this in a PR comment:

Re: datetime.date.fromisocalendar
Note that this now can 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.
Also, prior to this PR, the C version fromisocalendar would throw a ValueError while the Python version throws TypeError for issues with e.g, missing arguments.

@taleinat taleinat Jan 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, thanks, I didn't make the connection! I'll take another look at that.

]

for isocal in isocals:
Expand Down
40 changes: 17 additions & 23 deletions Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3022,25 +3022,24 @@ datetime_date_fromisoformat(PyTypeObject *type, PyObject *date_string)
}


static PyObject *
date_fromisocalendar(PyObject *cls, PyObject *args, PyObject *kw)
{
static char *keywords[] = {
"year", "week", "day", NULL
};
/*[clinic input]
@classmethod
datetime.date.fromisocalendar

int year, week, day;
if (PyArg_ParseTupleAndKeywords(args, kw, "iii:fromisocalendar",
keywords,
&year, &week, &day) == 0) {
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
PyErr_Format(PyExc_ValueError,
"ISO calendar component out of range");
year: int
week: int
day: int

}
Comment on lines -3020 to -3024

@taleinat taleinat Jan 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now I see that before this PR we have code in fromisocalendar() to raise ValueError rather than OverflowError. Raising a ValueError seems like better behavior to me. Also, changing this would be considered backwards-incompatible, would require NEWS and What's New entries, and couldn't be backported.

I think we should find a way to retain this behavior, even if that means not using Argument Clinic for this method right now.

return NULL;
}
int, int, int -> Construct a date from the ISO year, week number and weekday.

This is the inverse of the date.isocalendar() function
Comment thread
hauntsaninja marked this conversation as resolved.
Outdated
[clinic start generated code]*/

static PyObject *
datetime_date_fromisocalendar_impl(PyTypeObject *type, int year, int week,
int day)
/*[clinic end generated code: output=7b26e15115d24df6 input=f13669bbed6c8bb6]*/
{
// Year is bounded to 0 < year < 10000 because 9999-12-31 is (9999, 52, 5)
if (year < MINYEAR || year > MAXYEAR) {
PyErr_Format(PyExc_ValueError, "Year is out of range: %d", year);
Expand Down Expand Up @@ -3078,7 +3077,7 @@ date_fromisocalendar(PyObject *cls, PyObject *args, PyObject *kw)

ord_to_ymd(day_1 + day_offset, &year, &month, &day);

return new_date_subclass_ex(year, month, day, cls);
return new_date_subclass_ex(year, month, day, (PyObject *) type);
}


Expand Down Expand Up @@ -3503,12 +3502,7 @@ static PyMethodDef date_methods[] = {
DATETIME_DATE_FROMTIMESTAMP_METHODDEF
DATETIME_DATE_FROMORDINAL_METHODDEF
DATETIME_DATE_FROMISOFORMAT_METHODDEF

{"fromisocalendar", (PyCFunction)(void(*)(void))date_fromisocalendar,
METH_VARARGS | METH_KEYWORDS | METH_CLASS,
PyDoc_STR("int, int, int -> Construct a date from the ISO year, week "
"number and weekday.\n\n"
"This is the inverse of the date.isocalendar() function")},
DATETIME_DATE_FROMISOCALENDAR_METHODDEF

{"today", (PyCFunction)date_today, METH_NOARGS | METH_CLASS,
PyDoc_STR("Current date or datetime: same as "
Expand Down
65 changes: 64 additions & 1 deletion Modules/clinic/_datetimemodule.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.