You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/11/14 12:30:15 UTC
[GitHub] [arrow] 0x26res opened a new pull request, #14633: 18257 - pass back time types with correct type class
0x26res opened a new pull request, #14633:
URL: https://github.com/apache/arrow/pull/14633
https://issues.apache.org/jira/browse/ARROW-18257
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
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)
+
+
+@pytest.mark.parametrize("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)
+
+
+@pytest.mark.parametrize("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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14633:
URL: https://github.com/apache/arrow/pull/14633#issuecomment-1315100600
Benchmark runs are scheduled for baseline = 9f5a5bb7c1deeab7819cbcee29f3f8eefbd3f60b and contender = e3ea0cb2e3fffff8b644c39c1fa8c11b194cde7c. e3ea0cb2e3fffff8b644c39c1fa8c11b194cde7c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/8ff76cf7be3041c6b899122065f3bbae...7b80529ceb6547b78c212cbba8ad4795/)
[Finished :arrow_down:0.1% :arrow_up:0.03%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/e69d3ccfae2a456c82383ee8eb004ea5...65ce488652a44334a648e67fa5d3cfe6/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/c643a3e745d24d1d89fdc4cafa03ec30...769dda92f10a4cb5b0c964f4fe87f2ee/)
[Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e62a7640121a41d5ac9735895c40790a...e1f5e2e4fa994d8bb8acc4e049503663/)
Buildkite builds:
[Finished] [`e3ea0cb2` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1863)
[Finished] [`e3ea0cb2` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1885)
[Finished] [`e3ea0cb2` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1852)
[Finished] [`e3ea0cb2` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1877)
[Finished] [`9f5a5bb7` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1862)
[Finished] [`9f5a5bb7` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1884)
[Finished] [`9f5a5bb7` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1851)
[Finished] [`9f5a5bb7` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1876)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #14633: 18257 - pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14633:
URL: https://github.com/apache/arrow/pull/14633#issuecomment-1313619780
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] 0x26res commented on a diff in pull request #14633: 18257 - pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
0x26res commented on code in PR #14633:
URL: https://github.com/apache/arrow/pull/14633#discussion_r1021471636
##########
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:
@jorisvandenbossche I've noticed that date32 and date64 are missing here as well. But they would require more effort because Date32Type and Date64Type don't exist. I think I would need to introduced them in `types.pxi`. I'm not sure if this is necessary though, given these types don't have a custom field (unlike Time32Type which has a `unit`).
Also should the if statement in that function be in any specific order?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] 0x26res commented on a diff in pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
0x26res commented on code in PR #14633:
URL: https://github.com/apache/arrow/pull/14633#discussion_r1021568176
##########
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)
+
+
+@pytest.mark.parametrize("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)
+
+
+@pytest.mark.parametrize("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've removed these 2.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche merged pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche merged PR #14633:
URL: https://github.com/apache/arrow/pull/14633
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14633:
URL: https://github.com/apache/arrow/pull/14633#issuecomment-1313680267
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #14633: ARROW-18257: [Python] pass back time types with correct type class
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14633:
URL: https://github.com/apache/arrow/pull/14633#issuecomment-1313680219
https://issues.apache.org/jira/browse/ARROW-18257
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org