Skip to content

Add PyBytes_AsString and PyBytes_AsStringAndSize - #8462

Merged
sklam merged 12 commits into
numba:mainfrom
ianna:ianna/bytes-as-string
Apr 17, 2023
Merged

sklam merged 12 commits into
numba:mainfrom
ianna:ianna/bytes-as-string

Conversation

@ianna

@ianna ianna commented Sep 26, 2022

Copy link
Copy Markdown
Contributor

Resolve issue #8455:
add bytes_as_string and bytes_as_string_and_size

add bytes_as_string and bytes_as_string_and_size

@guilhermeleobas guilhermeleobas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @ianna, thanks for working on this issue. I only have one small comment regarding the return value of PyBytes_AsStringAndSize

Comment thread numba/core/pythonapi.py Outdated
Co-authored-by: Guilherme Leobas <guilhermeleobas@gmail.com>
@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 2 - In Progress labels Oct 4, 2022
@ianna

ianna commented Oct 4, 2022

Copy link
Copy Markdown
Contributor Author

@guilhermeleobas - Where shall I add a unit test for this PR? Thanks!

@guilhermeleobas

Copy link
Copy Markdown
Contributor

Hi @ianna, I don't think Numba has any tests that covers the Python API. Perhaps you can create a file for that. Let me know if you need any help

@guilhermeleobas guilhermeleobas added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Oct 4, 2022
ianna added 2 commits October 7, 2022 11:23
PyBytes_AsStringAndSize returns an integer
@ianna

ianna commented Oct 10, 2022

Copy link
Copy Markdown
Contributor Author

Hi @ianna, I don't think Numba has any tests that covers the Python API. Perhaps you can create a file for that. Let me know if you need any help

Thanks! It would be nice to have an example to follow. Could you, please, point me to one? Thanks!

@guilhermeleobas

guilhermeleobas commented Oct 10, 2022 •

Copy link
Copy Markdown
Contributor

HI @ianna, I wrote a small snippet of code which tests PyBytes_AsString. You can use it as a template to test PyBytes_AsStringAndSize. The idea is that we create a Python Bytes object from a unicode value and use it as argument to api.bytes_as_string.

Perhaps there's a better way to test the API (cc @sklam).

"""
Test Python API
"""


import ctypes
import unittest
from numba.core import types
from numba.core.extending import intrinsic
from numba import jit


@intrinsic
def _pyapi_bytes_as_string(typingctx, csrc, size):
    sig = types.voidptr(csrc, size)  # cstring == void*

    def codegen(context, builder, sig, args):
        [csrc, size] = args
        api = context.get_python_api(builder)
        b = api.bytes_from_string_and_size(csrc, size)
        return api.bytes_as_string(b)
    return sig, codegen


def PyBytes_AsString(uni):
    # test_PyBytes_AsString will call this function with a unicode type.
    # We then use the underlying buffer to create a PyBytes object and call the
    # PyBytes_AsString function with PyBytes object as argument
    return _pyapi_bytes_as_string(uni._data, uni._length)


class TestPythonAPI(unittest.TestCase):
    def test_PyBytes_AsString(self):
        cfunc = jit(nopython=True)(PyBytes_AsString)
        cstr = cfunc('hello')  # returns a cstring

        fn = ctypes.pythonapi.PyBytes_FromString
        fn.argtypes = [ctypes.c_void_p]
        fn.restype = ctypes.py_object
        obj = fn(cstr)

        # Use the cstring created from bytes_as_string to create a python
        # bytes object
        self.assertEqual(obj, b'hello')


if __name__ == '__main__':
    unittest.main()

Let me know if you have any questions.

@github-actions

Copy link
Copy Markdown

This pull request is marked as stale as it has had no activity in the past 3 months. Please respond to this comment if you're still interested in working on this. Many thanks!

@github-actions github-actions Bot added the stale Marker label for stale issues. label Mar 28, 2023
@guilhermeleobas

Copy link
Copy Markdown
Contributor

Hi @ianna, sorry for the really late review. I just saw you made progress in the PR. Could you merge Numba main branch into your branch? So that CI can run again

@guilhermeleobas guilhermeleobas removed the stale Marker label for stale issues. label Mar 28, 2023
@guilhermeleobas guilhermeleobas removed the 4 - Waiting on author Waiting for author to respond to review label Apr 5, 2023
@guilhermeleobas guilhermeleobas added the 4 - Waiting on reviewer Waiting for reviewer to respond to author label Apr 5, 2023

@guilhermeleobas guilhermeleobas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @ianna, could you add a test for PyBytes_AsStringAndSize? The idea for the test is similar to what you wrote:

  • Get a pointer to a c string and size by calling PyBytes_AsStringAndSize` inside an intrinsic
  • Use ctypes to call PyBytes_FromStringAndSize with the return values of the previous function
@intrinsic
def _pyapi_bytes_as_string_and_size(typingctx, csrc, size):
    # return a tuple containing the c-string and size
    retty = types.Tuple.from_types((csrc, size))
    sig = retty(csrc, size)

    def codegen(context, builder, sig, args):
        [csrc, size] = args
        pyapi = context.get_python_api(builder)
        b = pyapi.bytes_from_string_and_size(csrc, size)
        p_cstr = builder.alloca(pyapi.cstring)
        p_size = builder.alloca(pyapi.py_ssize_t)
        pyapi.bytes_as_string_and_size(b, p_cstr, p_size)

        cstr = builder.load(p_cstr)
        size = builder.load(p_size)
        tup = context.make_tuple(builder, sig.return_type, (cstr, size))
        return tup
    return sig, codegen


def PyBytes_AsStringAndSize(uni):
    return _pyapi_bytes_as_string_and_size(uni._data, uni._length)

@ianna
ianna marked this pull request as ready for review April 11, 2023 15:16
@ianna
ianna requested a review from guilhermeleobas April 11, 2023 15:17

@ianna ianna left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@guilhermeleobas - I think, I'm done with this PR. Both functions have tests. Please, have a look. Thanks!

@guilhermeleobas guilhermeleobas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @ianna, thanks. Just one last change.

Comment thread numba/tests/test_pythonapi.py Outdated
@guilhermeleobas guilhermeleobas added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Apr 12, 2023
Co-authored-by: Guilherme Leobas <guilhermeleobas@gmail.com>
@ianna
ianna requested a review from guilhermeleobas April 12, 2023 20:38

@guilhermeleobas guilhermeleobas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks @ianna

@guilhermeleobas guilhermeleobas added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on author Waiting for author to respond to review labels Apr 13, 2023
@stuartarchibald stuartarchibald added this to the Numba 0.58 RC milestone Apr 17, 2023
@sklam
sklam merged commit dadb3d1 into numba:main Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants