jorisvandenbossche commented on a change in pull request #7519:
URL: https://github.com/apache/arrow/pull/7519#discussion_r448619631
##########
File path: python/pyarrow/scalar.pxi
##########
@@ -16,1198 +16,745 @@
# under the License.
-_NULL = NA = None
+import collections
cdef class Scalar:
"""
- The base class for all array elements.
+ The base class for scalars.
"""
+ def __init__(self):
+ raise TypeError("Do not call {}'s constructor directly, use "
+ "pa.scalar() instead.".format(self.__class__.__name__))
-cdef class NullType(Scalar):
- """
- Singleton for null array elements.
- """
- # TODO rename this NullValue?
+ cdef void init(self, const shared_ptr[CScalar]& wrapped):
+ self.wrapped = wrapped
- def __cinit__(self):
- global NA
- if NA is not None:
- raise Exception('Cannot create multiple NAType instances')
+ @staticmethod
+ cdef wrap(const shared_ptr[CScalar]& wrapped):
+ cdef:
+ Scalar self
+ Type type_id = wrapped.get().type.get().id()
+
+ if type_id == _Type_NA:
+ return _NULL
+
+ typ = _scalar_classes[type_id]
+ self = typ.__new__(typ)
+ self.init(wrapped)
+
+ return self
+
+ cdef inline shared_ptr[CScalar] unwrap(self) nogil:
+ return self.wrapped
+
+ @property
+ def type(self):
+ return pyarrow_wrap_data_type(self.wrapped.get().type)
- self.type = null()
+ @property
+ def is_valid(self):
+ return self.wrapped.get().is_valid
def __repr__(self):
- return 'NULL'
+ return '<pyarrow.{}: {!r}>'.format(
+ self.__class__.__name__, self.as_py()
+ )
- def as_py(self):
- """
- Return None
- """
- return None
+ def __str__(self):
+ return str(self.as_py())
+
+ def equals(self, Scalar other):
+ return self.wrapped.get().Equals(other.unwrap().get()[0])
def __eq__(self, other):
- return NA
+ try:
+ if not isinstance(other, Scalar):
+ other = scalar(other, type=self.type)
+ return self.equals(other)
+ except (TypeError, ValueError, ArrowInvalid):
+ return NotImplemented
+
+ def __hash__(self):
+ cdef CScalarHash hasher
+ return hasher(self.wrapped)
+
+ def as_py(self):
+ raise NotImplementedError()
-_NULL = NA = NullType()
+_NULL = NA = None
-cdef class ArrayValue(Scalar):
+cdef class NullScalar(Scalar):
"""
- The base class for non-null array elements.
+ Concrete class for null scalars.
"""
- def __init__(self):
- raise TypeError("Do not call {}'s constructor directly, use array "
- "subscription instead."
- .format(self.__class__.__name__))
+ def __cinit__(self):
+ global NA
+ if NA is not None:
+ raise Exception('Cannot create multiple NAType instances')
+ self.init(shared_ptr[CScalar](new CNullScalar()))
- cdef void init(self, DataType type, const shared_ptr[CArray]& sp_array,
- int64_t index):
- self.type = type
- self.index = index
- self._set_array(sp_array)
+ def __init__(self):
+ pass
- cdef void _set_array(self, const shared_ptr[CArray]& sp_array):
- self.sp_array = sp_array
+ def __eq__(self, other):
+ return NA
Review comment:
Ah, looking at the actual diff of your latest changes, you didn't update
to return False, but to return True (for comparing with another Null value,
False for comparing with any other value of course, but I had the first case in
my mind when reacting ;))
So that basically does what I proposed above, but, I think we can still
remove the `__eq__` alltogether to use the base class implementation, which
should give the same behaviour.
----------------------------------------------------------------
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
