[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753:
URL: https://github.com/apache/arrow/pull/7753#discussion_r454500127
##
File path: ci/scripts/python_test.sh
##
@@ -29,4 +29,4 @@ export LD_LIBRARY_PATH=${ARROW_HOME}/lib:${LD_LIBRARY_PATH}
# Enable some checks inside Python itself
export PYTHONDEVMODE=1
-pytest -r s --pyargs pyarrow
+pytest -r s --pyargs pyarrow.tests.test_jvm
Review comment:
Yes :-)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753: URL: https://github.com/apache/arrow/pull/7753#discussion_r454494904 ## File path: python/pyarrow/jvm.py ## @@ -28,24 +28,38 @@ import pyarrow as pa -def jvm_buffer(arrowbuf): +class _JvmBufferNanny: +""" +An object that keeps a org.apache.arrow.memory.ArrowBuf's underlying +memory alive. +""" + +def __init__(self, jvm_buf): +self.jvm_buf = jvm_buf +self.jvm_buf.retain() + +def __del__(self): +self.jvm_buf.release() + + +def jvm_buffer(jvm_buf): """ Construct an Arrow buffer from org.apache.arrow.memory.ArrowBuf Parameters -- -arrowbuf: org.apache.arrow.memory.ArrowBuf +jvm_buf: org.apache.arrow.memory.ArrowBuf Arrow Buffer representation on the JVM. Returns --- pyarrow.Buffer Python Buffer that references the JVM memory. """ -address = arrowbuf.memoryAddress() -size = arrowbuf.capacity() -return pa.foreign_buffer(address, size, base=arrowbuf) +address = jvm_buf.memoryAddress() Review comment: I'm trying to improve this, hold on. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753: URL: https://github.com/apache/arrow/pull/7753#discussion_r454489525 ## File path: python/pyarrow/jvm.py ## @@ -28,24 +28,38 @@ import pyarrow as pa -def jvm_buffer(arrowbuf): +class _JvmBufferNanny: +""" +An object that keeps a org.apache.arrow.memory.ArrowBuf's underlying +memory alive. +""" + +def __init__(self, jvm_buf): +self.jvm_buf = jvm_buf +self.jvm_buf.retain() + +def __del__(self): +self.jvm_buf.release() + + +def jvm_buffer(jvm_buf): """ Construct an Arrow buffer from org.apache.arrow.memory.ArrowBuf Parameters -- -arrowbuf: org.apache.arrow.memory.ArrowBuf +jvm_buf: org.apache.arrow.memory.ArrowBuf Arrow Buffer representation on the JVM. Returns --- pyarrow.Buffer Python Buffer that references the JVM memory. """ -address = arrowbuf.memoryAddress() -size = arrowbuf.capacity() -return pa.foreign_buffer(address, size, base=arrowbuf) +address = jvm_buf.memoryAddress() Review comment: Do you mean the object could be collected from another thread? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753: URL: https://github.com/apache/arrow/pull/7753#discussion_r454441837 ## File path: python/pyarrow/jvm.py ## @@ -43,9 +43,14 @@ def jvm_buffer(arrowbuf): pyarrow.Buffer Python Buffer that references the JVM memory. """ +import jpype address = arrowbuf.memoryAddress() size = arrowbuf.capacity() -return pa.foreign_buffer(address, size, arrowbuf.asNettyBuffer()) + +# TODO: why can we not use arrowbuf as the base? Review comment: Well, it should be even easier with `pa.foreign_buffer`. Just pass a custom base object with the desired destructor... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753: URL: https://github.com/apache/arrow/pull/7753#discussion_r454438960 ## File path: python/pyarrow/jvm.py ## @@ -43,9 +43,14 @@ def jvm_buffer(arrowbuf): pyarrow.Buffer Python Buffer that references the JVM memory. """ +import jpype address = arrowbuf.memoryAddress() size = arrowbuf.capacity() -return pa.foreign_buffer(address, size, arrowbuf.asNettyBuffer()) + +# TODO: why can we not use arrowbuf as the base? Review comment: @xhochy Try [weakref.finalize](https://docs.python.org/3/library/weakref.html#weakref.finalize). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7753: ARROW-9385: [Python] finish Fix JPype tests
pitrou commented on a change in pull request #7753: URL: https://github.com/apache/arrow/pull/7753#discussion_r454374835 ## File path: python/pyarrow/jvm.py ## @@ -43,9 +43,14 @@ def jvm_buffer(arrowbuf): pyarrow.Buffer Python Buffer that references the JVM memory. """ +import jpype address = arrowbuf.memoryAddress() size = arrowbuf.capacity() -return pa.foreign_buffer(address, size, arrowbuf.asNettyBuffer()) + +# TODO: why can we not use arrowbuf as the base? Review comment: @rymurr Do you have the answer to this? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
