8000 BUG: fix PyArray_IsPythonScalar to detect bytes objects on Python 3 by immerrr · Pull Request #5256 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: fix PyArray_IsPythonScalar to detect bytes objects on Python 3 #5256

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

BUG: fix PyArray_IsPythonScalar to detect bytes objects on Python 3 #5256

wants to merge 1 commit into from

Conversation

immerrr
Copy link
Contributor
@immerrr immerrr commented Nov 2, 2014

This PR should fix PyArray_IsPythonScalar to return True for unibyte strings on Python 3.

As you can see here, unibyte strings don't pass PyString_Check on Python 3 and only PyBytes_Check will work there. On 2.6/2.7 PyBytes_Check is an alias for PyString_Check (or vice versa, I don't remember), so it should work too.

@juliantaylor
Copy link
Contributor

mh normally this is handled by our compat header, but apparently our public header was overlooked, thanks

@juliantaylor
Copy link
Contributor

I am a bit confused how this works with PyInt_Check also used by that macro. That function doesn't exist in python3 as far as I can tell.

@juliantaylor
Copy link
Contributor

hm the symbol probably coming from cython, so our headers need some more work to work freestanding

@immerrr
Copy link
Contributor Author
immerrr commented Nov 2, 2014

When you compile via cython, it adds some compat boilerplate beforehand:

#if PY_MAJOR_VERSION >= 3
  #define PyIntObject                  PyLongObject
  #define PyInt_Type                   PyLong_Type
  #define PyInt_Check(op)              PyLong_Check(op)

@juliantaylor
Copy link
Contributor

I have added your fix to gh-5257 which also should fix the integer issue. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@immerrr @juliantaylor
0