8000 DEP: Remove Python2 specific C module setup by sethtroisi · Pull Request #14590 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

DEP: Remove Python2 specific C module setup #14590

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

DEP: Remove Python2 specific C module setup #14590

wants to merge 1 commit into from

Conversation

sethtroisi
Copy link
Contributor

I'm excited about cleaning up all the Python2 specific code.

I broke out this small change first to make sure this can go in now and is a wanted changed.
If so there are ~700 instances of NPY_PY3K at least 50 of which contain real logic.

@mattip
Copy link
Member
mattip commented Sep 25, 2019

This will have to wait until 2020 when we drop python2 support. See the other PRs with the Py3K label. xref relevant comment

@sethtroisi
Copy link
Contributor Author

Okay, happy to wait :)
This making backport for 1.16 harder makes a lot of sense

jdufresne
jdufresne reviewed Sep 25, 2019
View reviewed changes
numpy/core/src/umath/_umath_tests.c.src

/* Initialization function for the module */
#if defined(NPY_PY3K)
#define RETVAL(x) x
Copy link
Contributor

Choose a reason for hiding this comment

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

RETVAL is now a pointless alias. Depending on how much should be done in one go, could potentially remove it as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm planning to clean it up in a 2nd pass.

@sethtroisi
Copy link
Contributor Author

It's after 2020! Can we start submitting Py3K fixes?

@charris
Copy link
Member
charris commented Jan 2, 2020

Yes, it is time. Lets take a look at the pending versions and take them in sequence. I think a rebase will fix the CircleCI failure.

@sethtroisi
Copy link
Contributor Author

I rebased, but circleci failed again during building devdocs, I don't touch any files there so I'm confused as to why.

@mattip
Copy link
Member
mattip commented Jan 3, 2020

xref #15227

@eric-wieser
Copy link
Member

I'm planning to clean it up in a 2nd pass.

This patch looks great to me, but let's remove RETVAL at the same time, since the macro makes no sense now that the ifdefs are gone.

@sethtroisi sethtroisi mentioned this pull request Jan 3, 2020
Merged
@sethtroisi
Copy link
Contributor Author
sethtroisi commented Jan 3, 2020 •
edited
Loading

@eric-wieser I cleaned up RETVAL and some other uses of PY_VERSION_HEX and NPY_PY3K in #15231 (which is a strict subset of these changes) it might be cleaner to just pull that (or to pull this then I can rebase #15231 immediately after)

@eric-wieser
Copy link
Member

Closing this one in favor of #15231, as suggested by @seberg

@eric-wieser eric-wieser closed this Jan 3, 2020
@sethtroisi sethtroisi deleted the remove_python2 branch January 3, 2020 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdufresne jdufresne jdufresne left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sethtroisi @mattip @charris @eric-wieser @jdufresne
0