jorisvandenbossche commented on code in PR #14633:
URL: https://github.com/apache/arrow/pull/14633#discussion_r1021546084


##########
python/pyarrow/public-api.pxi:
##########
@@ -97,6 +97,10 @@ cdef api object pyarrow_wrap_data_type(
         out = SparseUnionType.__new__(SparseUnionType)
     elif type.get().id() == _Type_DENSE_UNION:
         out = DenseUnionType.__new__(DenseUnionType)
+    elif type.get().id() == _Type_TIME32:
+        out = Time32Type.__new__(Time32Type)
+    elif type.get().id() == _Type_TIME64:
+        out = Time64Type.__new__(Time64Type)

Review Comment:
   > But they would require more effort because Date32Type and Date64Type don't 
exist. I'm not sure if this is necessary though, given these types don't have a 
custom field (unlike Time32Type which has a
   
   Yes, that's indeed because they don't have any custom logic or attributes 
(just like primitive numeric data types also don't have a custom subclass)
   
   > Also should the if statement in that function be in any specific order?
   
   I don't think so, current order looks fine



##########
python/pyarrow/tests/test_types.py:
##########
@@ -1131,3 +1131,24 @@ def test_hashing(items):
 
     for i, item in enumerate(items):
         assert container[item] == i
+
+
+def test_types_come_back_with_specific_type():
+    for arrow_type in get_many_types():
+        schema = pa.schema([pa.field("field_name", arrow_type)])
+        type_back = schema.field("field_name").type
+        assert type(type_back) is type(arrow_type)
+
+
[email protected]("unit", ["us", "ns"])
+def test_can_read_unit_from_time_64_array(unit: str):
+    array = pa.array([1, 2, 3], pa.time64(unit))
+    assert array.type.unit == unit
+    assert isinstance(array.type, pa.Time64Type)
+
+
[email protected]("unit", ["s", "ms"])
+def test_can_read_unit_from_time_32_array(unit: str):
+    array = pa.array([1, 2, 3], pa.time32(unit))
+    assert array.type.unit == unit
+    assert isinstance(array.type, pa.Time32Type)

Review Comment:
   I am not fully sure those two tests are needed. We already have 
`test_time32_units` and `test_time64_units` which cover the fact that those 
subclasses have a `unit` property. And the test you added above will ensure 
that this subclass is preserved when getting the type from an array of schema.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to