jorisvandenbossche commented on code in PR #13594:
URL: https://github.com/apache/arrow/pull/13594#discussion_r921928410
##########
python/pyarrow/scalar.pxi:
##########
@@ -902,10 +902,7 @@ cdef class ExtensionScalar(Scalar):
"""
Return this scalar as a Python object.
"""
- if self.value is None:
- return None
- else:
- return self.type.scalar_as_py(self.value)
+ return self.value if self.value is not None else None
Review Comment:
```suggestion
return self.value.as_py() if self.value is not None else None
```
? (it is a bit suspicious that the tests are not failing because of this)
##########
python/pyarrow/array.pxi:
##########
@@ -2771,6 +2771,11 @@ cdef class ExtensionArray(Array):
"""
return self.storage.to_numpy(**kwargs)
+ cdef getitem(self, int64_t i):
+ scalar = ExtensionScalar.wrap(GetResultValue(self.ap.GetScalar(i)))
+ return self.type.__arrow_ext_scalar_class__().from_storage(
+ self.type, scalar.value)
Review Comment:
Ah, reading further, I see you already defined a
`get_scalar_class_from_type` similarly as how it is done for arrays. In that
case I think it should actually be possible to update `Scalar.wrap` as well,
and then the above `getitem` override shouldn't be needed?
##########
docs/source/python/extending_types.rst:
##########
@@ -290,21 +290,25 @@ Custom scalar conversion
~~~~~~~~~~~~~~~~~~~~~~~~
If you want scalars of your custom extension type to convert to a custom type
when
-:meth:`ExtensionScalar.as_py()` is called, you can override the
:meth:`ExtensionType.scalar_as_py()`
+:meth:`ExtensionScalar.as_py()` is called, you can override the
:meth:`ExtensionType.as_py()`
Review Comment:
```suggestion
:meth:`ExtensionScalar.as_py()` is called, you can override the
:meth:`ExtensionScalar.as_py()`
```
##########
python/pyarrow/array.pxi:
##########
@@ -2771,6 +2771,11 @@ cdef class ExtensionArray(Array):
"""
return self.storage.to_numpy(**kwargs)
+ cdef getitem(self, int64_t i):
+ scalar = ExtensionScalar.wrap(GetResultValue(self.ap.GetScalar(i)))
+ return self.type.__arrow_ext_scalar_class__().from_storage(
+ self.type, scalar.value)
Review Comment:
The alternative would be to update `Scalar.wrap` / `pyarrow_wrap_scalar` to
do this already, similarly how this is done for Array wrapping (if you look for
how `__arrow_ext_class__` gets used, you might be able to do something similar,
although the scalar wrap code is structured a bit differently)
--
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.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
