Skip to content

Extension Module Compat Cleanup #29666

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

Merged
merged 3 commits into from
Nov 18, 2019
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Nov 17, 2019

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@gfyoung gfyoung added Clean Internals Related to non-user accessible pandas implementation labels Nov 17, 2019
@@ -17,7 +17,7 @@ The full license is in the LICENSE file, distributed with this software.
#define O_BINARY 0
#endif // O_BINARY

#if PY_VERSION_HEX >= 0x03060000 && defined(_WIN32)
#ifdef(_WIN32)
Copy link
Member

@gfyoung gfyoung Nov 17, 2019

Choose a reason for hiding this comment

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

I saw some compilation failures around this.

Can't we just do a pure clean and remove the PY_VERSiON_HEX check from that if statement?

@@ -38,13 +38,9 @@ PANDAS_INLINE int slice_get_indices(PyObject *s,
Py_ssize_t *stop,
Py_ssize_t *step,
Py_ssize_t *slicelength) {
#if PY_VERSION_HEX >= 0x03000000

return PySlice_GetIndicesEx(s, length, start, stop,
step, slicelength);
Copy link
Member

Choose a reason for hiding this comment

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

if we're returning this unchanged, can we just use this function directly and not define an alias? i guess the pypy thing above might complicate this?

Copy link
Member

@gfyoung gfyoung Nov 17, 2019

Choose a reason for hiding this comment

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

Unclear for me at this point, but makes sense to investigate in a follow-up.

@jreback
Copy link
Contributor

jreback commented Nov 17, 2019

lgtm, pending green.

@jreback jreback added this to the 1.0 milestone Nov 18, 2019
@jreback jreback merged commit b6d64d2 into pandas-dev:master Nov 18, 2019
@jreback
Copy link
Contributor

jreback commented Nov 18, 2019

thanks

Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
@WillAyd WillAyd deleted the ext-35-cleanup branch November 18, 2019 17:18
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants