Skip to content

bpo-20177: Migrate datetime.date.fromtimestamp to Argument Clinic #8535

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

Merged
merged 1 commit into from
Sep 24, 2018
Merged

bpo-20177: Migrate datetime.date.fromtimestamp to Argument Clinic #8535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Misc/ACKS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ Benjamin Hodgson
Joerg-Cyril Hoehle
Gregor Hoffleit
Chris Hoffman
Tim Hoffmann
Stefan Hoffmeister
Albert Hofkamp
Chris Hogan
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Documentation/2018-07-28-17-17-42.bpo-20177.cOZJWp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Migrate datetime.date.fromtimestamp to Argument Clinic. Patch by Tim Hoffmann.
37 changes: 20 additions & 17 deletions Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@
/*[clinic input]
module datetime
class datetime.datetime "PyDateTime_DateTime *" "&PyDateTime_DateTimeType"
class datetime.date "PyDateTime_Date *" "&PyDateTime_DateType"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=78142cb64b9e98bc]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=25138ad6a696b785]*/

#include "clinic/_datetimemodule.c.h"

Expand Down Expand Up @@ -2785,9 +2786,8 @@ date_new(PyTypeObject *type, PyObject *args, PyObject *kw)
return self;
}

/* Return new date from localtime(t). */
static PyObject *
date_local_from_object(PyObject *cls, PyObject *obj)
date_fromtimestamp(PyObject *cls, PyObject *obj)
{
struct tm tm;
time_t t;
Expand Down Expand Up @@ -2832,19 +2832,26 @@ date_today(PyObject *cls, PyObject *dummy)
return result;
}

/* Return new date from given timestamp (Python timestamp -- a double). */
/*[clinic input]
@classmethod
datetime.date.fromtimestamp

timestamp: object
/

Create a date from a POSIX timestamp.

The timestamp is a number, e.g. created via time.time(), that is interpreted
as local time.
[clinic start generated code]*/

static PyObject *
date_fromtimestamp(PyObject *cls, PyObject *args)
datetime_date_fromtimestamp(PyTypeObject *type, PyObject *timestamp)
/*[clinic end generated code: output=fd045fda58168869 input=eabb3fe7f40491fe]*/
{
PyObject *timestamp;
PyObject *result = NULL;

if (PyArg_ParseTuple(args, "O:fromtimestamp", &timestamp))
result = date_local_from_object(cls, timestamp);
return result;
return date_fromtimestamp((PyObject *) type, timestamp);
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't be better to just move the Argument Clinic declaration before date_fromtimestamp, and rename the later to datetime_date_fromtimestamp?

Copy link
Contributor Author

@timhoffm timhoffm Jul 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think that will work because date_fromtimestamp is the C API whereas datetime_date_fromtimestamp is the Python API and they differ in the type of the first argument. But maybe I'm overlooking something? These are just my first steps with CPython.

Copy link
Member

Choose a reason for hiding this comment

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

I meant that since date_fromtimestamp is used in just a single place, why not inline it? But instead of moving the body of date_fromtimestamp inside datetime_date_fromtimestamp, it is better to move the header of datetime_date_fromtimestamp (generated by Argument Clinic, together with the prepending Argument Clinic declaration) in place of the header of date_fromtimestamp. Both datetime_date_fromtimestamp and date_fromtimestamp are C functions, with virtually identical signatures.

Sorry if I'm not clear.

Copy link
Contributor Author

@timhoffm timhoffm Jul 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You mean, I should have just one function not the two? I don't think this works for the following reason.

I have to use datetime.date.fromtimestamp for the clinic. If I just use date.fromtimestampit will complain (Parent class or module does not exist).
With the clinic declaration, this results in exactly the signature

datetime_date_fromtimestamp(PyTypeObject *type, PyObject *timestamp)

However, I cannot use this function in the PyDateTime_CAPI declaration (l.6075). If I try that, the compiler complains

/home/tim/dev/cpython/Modules/_datetimemodule.c:6075:5: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
     datetime_date_fromtimestamp,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tim/dev/cpython/Modules/_datetimemodule.c:6075:5: note: (near initialization for ‘CAPI.Date_FromTimestamp’)

The function there is expected to have PyObject * as type, not PyTypeObject *. I don't know if that can be corrected, but reckon that it would be a breaking change to the CAPI which we do not want.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed that date_fromtimestamp()is not a new function, and it is used n other place. You could cast the function to the correct type as it was in thedate_methods` initialization:

(PyCFunction)datetime_date_fromtimestamp,

but the current code LGTM too.

}


/* Return new date from proleptic Gregorian ordinal. Raises ValueError if
* the ordinal is out of range.
*/
Expand Down Expand Up @@ -3184,11 +3191,7 @@ date_reduce(PyDateTime_Date *self, PyObject *arg)
static PyMethodDef date_methods[] = {

/* Class methods: */

{"fromtimestamp", (PyCFunction)date_fromtimestamp, METH_VARARGS |
METH_CLASS,
PyDoc_STR("timestamp -> local date from a POSIX timestamp (like "
"time.time()).")},
DATETIME_DATE_FROMTIMESTAMP_METHODDEF

{"fromordinal", (PyCFunction)date_fromordinal, METH_VARARGS |
METH_CLASS,
Expand Down
14 changes: 13 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.