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/09/05 18:51:20 UTC
[GitHub] [arrow] kshitij12345 opened a new pull request, #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
kshitij12345 opened a new pull request, #14048:
URL: https://github.com/apache/arrow/pull/14048
```python
table = pa.table({'a': [None, 1], 'b': [None, True]})
new_schema = pa.schema([pa.field("a", "int64", nullable=True), pa.field("b", "bool", nullable=False)])
casted = table.cast(new_schema)
```
Now leads to
```
RuntimeError: Casting a nullable field 'b' to non-nullable
```
--
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 #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14048:
URL: https://github.com/apache/arrow/pull/14048#discussion_r965595748
##########
python/pyarrow/table.pxi:
##########
@@ -3401,6 +3401,9 @@ cdef class Table(_PandasConvertible):
.format(self.schema.names, target_schema.names))
for column, field in zip(self.itercolumns(), target_schema):
+ if column.null_count > 0 and not field.nullable:
+ raise RuntimeError("Casting field {!r} with null values to non-nullable"
Review Comment:
I think this can be a ValueError
##########
python/pyarrow/table.pxi:
##########
@@ -3401,6 +3401,9 @@ cdef class Table(_PandasConvertible):
.format(self.schema.names, target_schema.names))
for column, field in zip(self.itercolumns(), target_schema):
+ if column.null_count > 0 and not field.nullable:
Review Comment:
```suggestion
if not field.nullable and column.null_count > 0:
```
Switching the order will avoid checking the null_count (potentially expensive) if the field is nullable (which will be the most common case, since this is the default)
--
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] kshitij12345 commented on pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
kshitij12345 commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1240362768
@jorisvandenbossche Have addressed the review. Thanks! PTAL :)
--
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 pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237771956
Yeah, that can indeed be slower, so that is certainly a trade-off to make.
An array can store an optional `null_count`, which is cached once known, so getting the null count _can_be fast (depending on whether it is already available).
--
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] kshitij12345 commented on pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
kshitij12345 commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237391936
cc: @AlenkaF @jorisvandenbossche
--
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 pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237752880
We should probably check if a field actually has nulls, instead of only the nullability flag of the field?
Meaning: I think it should still be possible to cast a nullable field to non-nullable, _if_ that field has no nulls in it?
--
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 #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1240828276
Benchmark runs are scheduled for baseline = 43670af02f0913580fd20e26006fd550d6fdf2da and contender = df121b7feec92464a4e97fe535a864537a16be1b. df121b7feec92464a4e97fe535a864537a16be1b 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/f0981389d6cb4cd8ae00f4bbb90dc68e...14f93d2b2adf4adb8f3206d35a69053d/)
[Failed :arrow_down:0.37% :arrow_up:0.1%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/e6dbed5d6ca4421bbd7d08c1a718b193...3d8182f4bb6046499f4121917ea182ee/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/771338932108463c8a582147b7e2c079...eb4c5b300a4a4832960d2c5e77b34222/)
[Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c75ca59f5e98486399224a237be66dd1...7a40e73a52554100938cd0ee4ebd3884/)
Buildkite builds:
[Finished] [`df121b7f` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1438)
[Failed] [`df121b7f` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1454)
[Failed] [`df121b7f` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1438)
[Finished] [`df121b7f` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1451)
[Finished] [`43670af0` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1437)
[Failed] [`43670af0` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1453)
[Failed] [`43670af0` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1437)
[Finished] [`43670af0` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1450)
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 #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237389776
: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 #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237389760
https://issues.apache.org/jira/browse/ARROW-16651
--
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] kshitij12345 commented on pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
kshitij12345 commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1238354814
@jorisvandenbossche Have updated to check the `null_count`. PTAL :)
--
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 #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche merged PR #14048:
URL: https://github.com/apache/arrow/pull/14048
--
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] kshitij12345 commented on a diff in pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
kshitij12345 commented on code in PR #14048:
URL: https://github.com/apache/arrow/pull/14048#discussion_r965622471
##########
python/pyarrow/table.pxi:
##########
@@ -3401,6 +3401,9 @@ cdef class Table(_PandasConvertible):
.format(self.schema.names, target_schema.names))
for column, field in zip(self.itercolumns(), target_schema):
+ if column.null_count > 0 and not field.nullable:
Review Comment:
Makes sense! Thanks!
--
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] kshitij12345 commented on pull request #14048: ARROW-16651 : [Python] Casting Table to new schema ignores nullability of fields
Posted by GitBox <gi...@apache.org>.
kshitij12345 commented on PR #14048:
URL: https://github.com/apache/arrow/pull/14048#issuecomment-1237756018
Wouldn't that be much slower if we have to check if the field has null in it's data or is there meta-data stored around that?
--
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