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

Ignore .pxd inline functions that are not used #2051

Open
mattip opened this Issue Dec 21, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@mattip
Contributor

mattip commented Dec 21, 2017

still working out the details, glad to hear any comments and note my analysis of the interaction between cython and PyPy may be mistaken.

Pandas recently added this line in changeset pandas-dev/pandas@f7f33a:

from cpython.datetime cimport PyTZInfo_Check

which seemed to be enough to cause cython to emit code from cython's datetime.pxd that PyPy does not support. It seems PyDateTime_Time and friends are opaque structures on PyPy. PyPy does not even implement the hastzinfo and tzinfo fields, which is probably wrong. But could cython use the macros PyDateTime_DATE_GET_MICROSECOND and friends instead of directly accessing fields in the structs?

The errors look like

pandas/_libs/tslib.c: In function ‘__pyx_f_7cpython_8datetime_time_tzinfo’:
pandas/_libs/tslib.c:71199:46: error: ‘PyDateTime_Time {aka struct <anonymous>}’ has no member named ‘hastzinfo’
   __pyx_t_1 = (((PyDateTime_Time *)__pyx_v_o)->hastzinfo != 0);
                                              ^
In file included from /home/matti/pypy_stuff/pypy2-v5.10/include/Python.h:81:0,
                 from pandas/_libs/tslib.c:4:
pandas/_libs/tslib.c:71210:61: error: ‘PyDateTime_Time {aka struct <anonymous>}’ has no member named ‘tzinfo’
     __Pyx_INCREF(((PyObject *)((PyDateTime_Time *)__pyx_v_o)->tzinfo));
                                                             ^
pandas/_libs/tslib.c: In function ‘__pyx_f_7cpython_8datetime_timedelta_days’:
pandas/_libs/tslib.c:71864:44: error: ‘PyDateTime_Delta {aka struct <anonymous>}’ has no member named ‘days’
   __pyx_r = ((PyDateTime_Delta *)__pyx_v_o)->days;
                                            ^
pandas/_libs/tslib.c: In function ‘__pyx_f_7cpython_8datetime_timedelta_seconds’:
pandas/_libs/tslib.c:71901:44: error: ‘PyDateTime_Delta {aka struct <anonymous>}’ has no member named ‘seconds’
   __pyx_r = ((PyDateTime_Delta *)__pyx_v_o)->seconds;
                                            ^
pandas/_libs/tslib.c: In function ‘__pyx_f_7cpython_8datetime_timedelta_microseconds’:
pandas/_libs/tslib.c:71935:44: error: ‘PyDateTime_Delta {aka struct <anonymous>}’ has no member named ‘microseconds’
   __pyx_r = ((PyDateTime_Delta *)__pyx_v_o)->microseconds;
@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Dec 21, 2017

Contributor

Those come from inline functions. I think we should try to exclude inline functions that are defined in .pxd files but not used anywhere.

I'll rename the ticket accordingly since this is the main issue here. For declarations that are actually used, it's up to the users to care about compatibility (if Cython cannot easily cover it up in a non-ambiguous way).

Contributor

scoder commented Dec 21, 2017

Those come from inline functions. I think we should try to exclude inline functions that are defined in .pxd files but not used anywhere.

I'll rename the ticket accordingly since this is the main issue here. For declarations that are actually used, it's up to the users to care about compatibility (if Cython cannot easily cover it up in a non-ambiguous way).

@scoder scoder changed the title from pypy does not have fields in its DateTime structs to Ignore .pxd inline functions that are not used Dec 21, 2017

@mattip

This comment has been minimized.

Show comment
Hide comment
@mattip

mattip Dec 22, 2017

Contributor

FWIW, both PyPy2 and PyPy3 as well as CPython3 have PyDateTime_DELTA_GET_* macros. The ``tzinfo part seems like a PyPy bug

Contributor

mattip commented Dec 22, 2017

FWIW, both PyPy2 and PyPy3 as well as CPython3 have PyDateTime_DELTA_GET_* macros. The ``tzinfo part seems like a PyPy bug

@mattip

This comment has been minimized.

Show comment
Hide comment
@mattip

mattip Mar 20, 2018

Contributor

The original issue was a PyPy bug that has been fixed on HEAD. I would suggest closing this and, if desired, opening a separate issue for unused inline functions or at least removing the PyPy tag

Contributor

mattip commented Mar 20, 2018

The original issue was a PyPy bug that has been fixed on HEAD. I would suggest closing this and, if desired, opening a separate issue for unused inline functions or at least removing the PyPy tag

@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Mar 20, 2018

Contributor

Thanks for following up. There is no PyPy tag, and I think the title describes the problem correctly, so I'd say it can stay open.

Contributor

scoder commented Mar 20, 2018

Thanks for following up. There is no PyPy tag, and I think the title describes the problem correctly, so I'd say it can stay open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment