Skip to content

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
#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?

#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