8000 MAINT: Remove NPY_PY3K constant by sethtroisi · Pull Request #15304 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

MAINT: Remove NPY_PY3K constant #15304

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
Merged
merged 1 commit into from
Jan 10, 2020
Merged

MAINT: Remove NPY_PY3K constant #15304

merged 1 commit into from
Jan 10, 2020

Conversation

sethtroisi
Copy link
Contributor

After this PR NPY_PY3K should only appear in 'npy_3kcompat.h' and
'doc/Py3K.rst.txt'

I've had a lot of fun removing python2 code and wanted to say thanks to all of the wonderful maintainers who have answered my questions, promptly commented on my pulls, and provided advice along the way. numpy is amazing!
numpy_thanks

@sethtroisi
MAINT: Remove NPY_PY3K
3142f0f 
After this PR NPY_PY3K should only appear in 'npy_3kcompat.h' and
'doc/Py3K.rst.txt'
sethtroisi
sethtroisi commented Jan 9, 2020
View reviewed changes
numpy/core/src/multiarray/descriptor.c
@@ -1309,18 +1309,9 @@ _convert_from_type(PyObject *obj) {
if (PyType_IsSubtype(typ, &PyGenericArrType_Type)) {
return PyArray_DescrFromTypeObject(obj);
}
#if !defined(NPY_PY3K)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This avoided being cleaned up because of merge conflicts along the way.

Copy link
Member

Choose a reason for hiding this comment

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

Guilty

@mattip mattip merged commit 0aa8370 into numpy:master Jan 10, 2020
@mattip
Copy link
Member
mattip commented Jan 10, 2020

Thanks @sethtroisi

@sethtroisi sethtroisi deleted the remove_NPY_PY3K branch January 10, 2020 00:35
@seberg
Copy link
Member
seberg commented Jan 10, 2020

Hmmm, what should happen with the stuff in numpy/core/include/numpy/npy_3kcompat.h?

@eric-wieser
Copy link
Member

Is that file a public header? How do downstream libraries get their NPY_PY3K definition if so?

@sethtroisi
Copy link
Contributor Author

(sorry if this is obvious) my assumption was that anyone building downstream c would include numpy/core/include/numpy/npy_3kcompat.h:17 which adds

#ifndef NPY_PY3K
#define NPY_PY3K 1
#endif

But this was only an assumption.

@sethtroisi sethtroisi mentioned this pull request Jan 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@sethtroisi @mattip @seberg @eric-wieser
0