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/05/09 23:35:15 UTC
[GitHub] [arrow] JabariBooker opened a new pull request, #13109: Arrow 15365: [Python] Expose full cast options in the pyarrow.compute.cast function
JabariBooker opened a new pull request, #13109:
URL: https://github.com/apache/arrow/pull/13109
Added CastOptions to pc.cast and related relate cast functions
--
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 #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r885486000
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,6 +1702,18 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions()
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
Review Comment:
I think we should also still test with a non-default `pc.CastOptions`, to ensure we are really testing that the options object is passed through (the test below `pc.cast(arr, 'int8', options=options)` would also pass if `options` was not specified)
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r904379120
##########
python/pyarrow/_compute.pyx:
##########
@@ -2198,7 +2198,14 @@ cdef class Expression(_Weakrefable):
-------
cast : Expression
"""
- options = CastOptions.safe(ensure_type(type))
+ if (safe is not None) and (options is not None):
+ raise ValueError(
+ "Must past only a value for 'safe' or only a value for 'options'")
Review Comment:
```suggestion
"Must pass only a value for 'safe' or only a value for 'options'")
```
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,13 +1702,33 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions(pa.int8())
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
+ assert pc.cast(arr, 'int8', options=options) == pa.array(
+ [1, 2, 3, 4], type='int8')
+
arr = pa.array([2 ** 63 - 1], type='int64')
+ allow_overflow_options = pc.CastOptions(
+ pa.int32(), allow_int_overflow=True)
with pytest.raises(pa.ArrowInvalid):
pc.cast(arr, 'int32')
assert pc.cast(arr, 'int32', safe=False) == pa.array([-1], type='int32')
+ assert pc.cast(arr, 'int32', options=allow_overflow_options) == pa.array(
+ [-1], type='int32')
Review Comment:
Now that I look at it…should we make the `type` an optional argument too? It's redundant with options (either you pass `type` and `safe`, or you pass `options`)
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r872766220
##########
python/pyarrow/array.pxi:
##########
@@ -895,7 +895,7 @@ cdef class Array(_PandasConvertible):
result = self.ap.Diff(deref(other.ap))
return frombytes(result, safe=True)
- def cast(self, object target_type, safe=True):
+ def cast(self, object target_type, safe=True, options=None):
Review Comment:
Yeah, exactly - we shouldn't allow passing both ideally since they're somewhat redundant
--
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 #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r875567732
##########
python/pyarrow/compute.py:
##########
@@ -369,10 +371,14 @@ def cast(arr, target_type, safe=True):
"""
if target_type is None:
raise ValueError("Cast target type must not be None")
- if safe:
- options = CastOptions.safe(target_type)
- else:
- options = CastOptions.unsafe(target_type)
+ if (safe is not None) and (options is not None):
+ raise ValueError(
+ "Must past only a value for 'safe' or only a value for 'options'")
+ if options is None:
+ if safe:
Review Comment:
We will need some handling here to interpret `safe=None` as `safe=True` (now it will be interpreted as False, which explains the test failures)
##########
python/pyarrow/array.pxi:
##########
@@ -905,14 +905,16 @@ cdef class Array(_PandasConvertible):
----------
target_type : DataType
Type to cast array to.
- safe : boolean, default True
+ safe : boolean, default None
Review Comment:
Although we changed it in the signature, I would keep it here as "True", because that is still the actual default behaviour if you don't specify neither of the params.
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r900060000
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,13 +1702,37 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions(pa.int8())
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
+ assert pc.cast(arr, 'int8', options=options) == pa.array(
+ [1, 2, 3, 4], type='int8')
+
arr = pa.array([2 ** 63 - 1], type='int64')
+ options = pc.CastOptions(pa.int32())
+ allow_overflow_options = pc.CastOptions(
+ pa.int32(), allow_int_overflow=True)
with pytest.raises(pa.ArrowInvalid):
pc.cast(arr, 'int32')
+ with pytest.raises(pa.ArrowInvalid):
+ pc.cast(arr, 'int32', options)
Review Comment:
What I mean is that even though we're passing `options`, it's getting ignored/interpreted as `safe=True` (and fails due to a value being out of range). It looks like a weird thing to test, basically.
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r907365385
##########
python/pyarrow/compute.py:
##########
@@ -367,12 +369,18 @@ def cast(arr, target_type, safe=True):
-------
casted : Array
"""
- if target_type is None:
- raise ValueError("Cast target type must not be None")
- if safe:
- options = CastOptions.safe(target_type)
- else:
- options = CastOptions.unsafe(target_type)
+ safe_vars_passed = (safe is not None) or (target_type is not None)
+
+ if safe_vars_passed and (options is not None):
+ raise ValueError("Must either pass values for 'target_type' and 'safe'"
+ " or pass a value for 'options'")
+
+ if options is None:
+ pa.types.lib.ensure_type(target_type)
Review Comment:
```suggestion
type = pa.types.lib.ensure_type(target_type)
```
##########
python/pyarrow/_compute.pyx:
##########
@@ -2198,7 +2198,18 @@ cdef class Expression(_Weakrefable):
-------
cast : Expression
"""
- options = CastOptions.safe(ensure_type(type))
+ safe_vars_passed = (safe is not None) or (type is not None)
+
+ if safe_vars_passed and (options is not None):
+ raise ValueError("Must either pass values for 'type' and 'safe' or pass a "
+ "value for 'options'")
+
+ if options is None:
+ ensure_type(type, allow_none=False)
Review Comment:
```suggestion
type = ensure_type(type, allow_none=False)
```
--
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 #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13109:
URL: https://github.com/apache/arrow/pull/13109#issuecomment-1121687815
https://issues.apache.org/jira/browse/ARROW-15365
--
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] JabariBooker commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
JabariBooker commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r872684533
##########
python/pyarrow/array.pxi:
##########
@@ -895,7 +895,7 @@ cdef class Array(_PandasConvertible):
result = self.ap.Diff(deref(other.ap))
return frombytes(result, safe=True)
- def cast(self, object target_type, safe=True):
+ def cast(self, object target_type, safe=True, options=None):
Review Comment:
Is this to clear up usage (i.e. we want the user to either pass a value for `safe` or `options` but never both)? As it is now, if a `CastOptions` is passed, the safe parameter is just ignored.
##########
python/pyarrow/array.pxi:
##########
@@ -895,7 +895,7 @@ cdef class Array(_PandasConvertible):
result = self.ap.Diff(deref(other.ap))
return frombytes(result, safe=True)
- def cast(self, object target_type, safe=True):
+ def cast(self, object target_type, safe=True, options=None):
Review Comment:
Is this to clear up usage (i.e. we want the user to either pass a value for `safe` or `options` but never both)? As it is now, if a `CastOptions` is passed, the `safe` parameter is just ignored.
--
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] JabariBooker commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
JabariBooker commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r905306348
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,13 +1702,33 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions(pa.int8())
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
+ assert pc.cast(arr, 'int8', options=options) == pa.array(
+ [1, 2, 3, 4], type='int8')
+
arr = pa.array([2 ** 63 - 1], type='int64')
+ allow_overflow_options = pc.CastOptions(
+ pa.int32(), allow_int_overflow=True)
with pytest.raises(pa.ArrowInvalid):
pc.cast(arr, 'int32')
assert pc.cast(arr, 'int32', safe=False) == pa.array([-1], type='int32')
+ assert pc.cast(arr, 'int32', options=allow_overflow_options) == pa.array(
+ [-1], type='int32')
Review Comment:
Agreed. As it is now, you effectively pass the "to" type twice and error is returned if your value for `type` doesn't match that of `options.target_type`.
--
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] JabariBooker commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
JabariBooker commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r899462686
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,13 +1702,37 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions(pa.int8())
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
+ assert pc.cast(arr, 'int8', options=options) == pa.array(
+ [1, 2, 3, 4], type='int8')
+
arr = pa.array([2 ** 63 - 1], type='int64')
+ options = pc.CastOptions(pa.int32())
+ allow_overflow_options = pc.CastOptions(
+ pa.int32(), allow_int_overflow=True)
with pytest.raises(pa.ArrowInvalid):
pc.cast(arr, 'int32')
+ with pytest.raises(pa.ArrowInvalid):
+ pc.cast(arr, 'int32', options)
Review Comment:
This was intended to ensure that passing a `CastOptions` object like this would fail in the same way `safe=options` would.
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r882041136
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,6 +1702,18 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions()
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
Review Comment:
Thanks for testing the error cases, can we also test that the non-error case now works with cast options?
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r871757065
##########
python/pyarrow/array.pxi:
##########
@@ -895,7 +895,7 @@ cdef class Array(_PandasConvertible):
result = self.ap.Diff(deref(other.ap))
return frombytes(result, safe=True)
- def cast(self, object target_type, safe=True):
+ def cast(self, object target_type, safe=True, options=None):
Review Comment:
The redundancy between `safe` and `options` isn't great. I wonder if we can have `safe=None`, and then raise an exception if passing both `safe` and `options`?
--
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] JabariBooker commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
JabariBooker commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r872684533
##########
python/pyarrow/array.pxi:
##########
@@ -895,7 +895,7 @@ cdef class Array(_PandasConvertible):
result = self.ap.Diff(deref(other.ap))
return frombytes(result, safe=True)
- def cast(self, object target_type, safe=True):
+ def cast(self, object target_type, safe=True, options=None):
Review Comment:
Is this clear up usage (i.e. we want the user to either pass a value for `safe` or `options` but never both)? As it is now, if an `CastOptions` is passed, the safe parameter is just ignored.
--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r898399806
##########
python/pyarrow/tests/test_compute.py:
##########
@@ -1702,13 +1702,37 @@ def test_logical():
def test_cast():
+ arr = pa.array([1, 2, 3, 4], type='int64')
+ options = pc.CastOptions(pa.int8())
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, target_type=None)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=True, options=options)
+
+ with pytest.raises(ValueError):
+ pc.cast(arr, 'int32', safe=False, options=options)
+
+ assert pc.cast(arr, 'int8', options=options) == pa.array(
+ [1, 2, 3, 4], type='int8')
+
arr = pa.array([2 ** 63 - 1], type='int64')
+ options = pc.CastOptions(pa.int32())
+ allow_overflow_options = pc.CastOptions(
+ pa.int32(), allow_int_overflow=True)
with pytest.raises(pa.ArrowInvalid):
pc.cast(arr, 'int32')
+ with pytest.raises(pa.ArrowInvalid):
+ pc.cast(arr, 'int32', options)
Review Comment:
Ah - this is misleading, this is passing `safe=options`, is that what you meant to test?
--
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 #13109: Arrow 15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13109:
URL: https://github.com/apache/arrow/pull/13109#issuecomment-1121687285
<!--
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] lidavidm commented on a diff in pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13109:
URL: https://github.com/apache/arrow/pull/13109#discussion_r906005079
##########
python/pyarrow/_compute.pyx:
##########
@@ -2198,7 +2198,20 @@ cdef class Expression(_Weakrefable):
-------
cast : Expression
"""
- options = CastOptions.safe(ensure_type(type))
+ safe_vars_passed = (safe is not None) or (type is not None)
+
+ if safe_vars_passed and (options is not None):
+ raise ValueError("Must either pass values for 'type' and 'safe' or pass a "
+ "value for 'options'")
+
+ if options is None:
+ if type is None:
+ raise ValueError(
Review Comment:
Also, we can just call `ensure_type(type, allow_none=False)`. That has the added benefit that it'll take string type arguments like `"int8"` like it did before. (Though I guess maybe CastOptions does that same conversion.)
##########
python/pyarrow/_compute.pyx:
##########
@@ -2198,7 +2198,20 @@ cdef class Expression(_Weakrefable):
-------
cast : Expression
"""
- options = CastOptions.safe(ensure_type(type))
+ safe_vars_passed = (safe is not None) or (type is not None)
+
+ if safe_vars_passed and (options is not None):
+ raise ValueError("Must either pass values for 'type' and 'safe' or pass a "
+ "value for 'options'")
+
+ if options is None:
+ if type is None:
+ raise ValueError(
Review Comment:
nit, but usually Python uses TypeError for things like missing arguments (here and above)
--
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] lidavidm commented on pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13109:
URL: https://github.com/apache/arrow/pull/13109#issuecomment-1135141706
I would still like to see a test.
--
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] lidavidm merged pull request #13109: ARROW-15365: [Python] Expose full cast options in the pyarrow.compute.cast function
Posted by GitBox <gi...@apache.org>.
lidavidm merged PR #13109:
URL: https://github.com/apache/arrow/pull/13109
--
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