paleolimbot commented on code in PR #502:
URL: https://github.com/apache/arrow-nanoarrow/pull/502#discussion_r1630003589
##########
python/tests/test_c_buffer.py:
##########
@@ -362,3 +363,47 @@ def test_c_buffer_bitmap_from_iterable():
builder.write_elements([True, False])
with pytest.raises(NotImplementedError, match="Append to bitmap"):
builder.write_elements([True])
+
+
+def test_c_buffer_from_timestamp_iterable():
Review Comment:
I am not sure that one should be able to do `na.c_buffer([1, 2, 3],
na.timestamp("ms"))` without getting an error (a timestamp is a valid *array*
type but not really a valid *buffer* type). I think that the `buffer_format`
vs. `storage_buffer_format` change will take care of this.
##########
python/tests/test_c_buffer.py:
##########
@@ -259,8 +260,8 @@ def test_c_buffer_from_iterable():
# An Arrow type whose storage type is not the same as its top-level
# type will error.
- with pytest.raises(ValueError, match="Can't create buffer"):
- na.c_buffer([1, 2, 3], na.date32())
+ # with pytest.raises(ValueError, match="Can't create buffer"):
+ # na.c_buffer([1, 2, 3], na.duration("s"))
Review Comment:
```suggestion
with pytest.raises(ValueError, match="Can't create buffer"):
na.c_buffer([1, 2, 3], na.dictionary(na.int32(), na.string()))
```
(I think this should trigger the intended code path without requiring the
new construction features!)
##########
python/tests/test_c_array.py:
##########
@@ -514,3 +516,71 @@ def test_c_array_from_buffers_validation():
validation_level=validation_level,
)
assert c_array.length == 2
+
+
+def test_c_array_timestamp_seconds():
+ d1 = int(round(datetime(1970, 1, 1).timestamp()))
+ d2 = int(round(datetime(1985, 12, 31).timestamp()))
+ d3 = int(round(datetime(2005, 3, 4).timestamp()))
+ c_array = na.c_array([d1, d2, d3], na.timestamp("s"))
Review Comment:
In addition to `na.c_array([d1, d2, d3], na.timestamp("s"))`, I believe that
creating this from a pybuffer (e.g., numpy) should work too. Perhaps we should
also check `na.c_array(array.array("i", [d1, d2, d3]), na.timestamp("s"))`?
##########
python/tests/test_array.py:
##########
@@ -354,3 +355,64 @@ def test_array_inspect(capsys):
array.inspect()
captured = capsys.readouterr()
assert captured.out.startswith("<ArrowArray struct<col0: int32")
+
+
+def test_timestamp_array():
+ d1 = int(round(datetime(1985, 12, 31, 0, 0,
tzinfo=timezone.utc).timestamp() * 1e3))
+ d2 = int(round(datetime(2005, 3, 4, 0, 0, tzinfo=timezone.utc).timestamp()
* 1e3))
+ array = na.Array([d1, d2], na.timestamp("ms"))
+ assert list(array.to_pysequence()) == [d1, d2]
Review Comment:
I am not sure that this should be true: returning `d1` and `d2` here by
default looses information (the datetime-ness and unit). I think that this
happens because `buffer_format` is not `None` (i.e., the
`storage_buffer_format` change would fix this). (Also true for following tests)
--
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]
