You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "voidstar69 (via GitHub)" <gi...@apache.org> on 2023/11/11 10:47:48 UTC
[PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
voidstar69 opened a new pull request, #38680:
URL: https://github.com/apache/arrow/pull/38680
### What changes are included in this PR?
Make Arrow arrays of scalar type T implement the same semantic contract as IReadOnlyList<T?>.
Note that this PR does not include similar support for ICollection<T?>. I could add that support in this PR or a future PR.
### Are these changes tested?
This PR includes unit tests of the implemented IReadOnlyList<T?> methods.
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "voidstar69 (via GitHub)" <gi...@apache.org>.
voidstar69 commented on PR #38680:
URL: https://github.com/apache/arrow/pull/38680#issuecomment-1809245756
> I was thinking this would include all scalar arrays, e.g. TimestampArray : IReadOnlyList<DateTime?>, StringArray : IReadOnlyList, TimeXXArray : IReadOnlyList<TimeOnly?>, DateXXArray : IReadOnlyList<DateOnly?>, IReadOnlyList<DateTime?>, BooleanArray : IReadOnlyList<bool?>, BinaryArray : IReadOnlyList<byte[]>.
>
> The original issue also suggests implementing ICollection.
>
> Would you file followup work items so that this one can be marked as closed if checked-in as-is?
I have implemented this for all the scalar arrays in your example. Are there any more scalar arrays that need to support `IReadOnlyList`?
For `TimestampArray` it seemed more natural for it to implement `IReadOnlyList<DateTimeOffset?>`.
Note that I implemented the various `GetEnumerator` methods in two distinct ways, a verbose for-loop and a concise LINQ statement. Let me know which you think is better and I will standardise on that form.
I created a followup issue #38692 for adding support for `ICollection<T?>`.
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on PR #38680:
URL: https://github.com/apache/arrow/pull/38680#issuecomment-1810948386
> Note that I implemented the various `GetEnumerator` methods in two distinct ways, a verbose for-loop and a concise LINQ statement. Let me know which you think is better and I will standardise on that form.
While concision is very nice, a for-loop allows inlining in a way that delegates don't (or don't yet at least) and I think that performance is more important than concision code in infrastructure.
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38680:
URL: https://github.com/apache/arrow/pull/38680#issuecomment-1806781599
:warning: GitHub issue #38348 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38680:
URL: https://github.com/apache/arrow/pull/38680#issuecomment-1817949905
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 96e62d86abe8a39a7fc20af7236bd43a9fd7f2da.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/18829165786) has more details.
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on PR #38680:
URL: https://github.com/apache/arrow/pull/38680#issuecomment-1808630721
I was thinking this would include all scalar arrays, e.g. TimestampArray : IReadOnlyList<DateTime?>, StringArray : IReadOnlyList<string>, TimeXXArray : IReadOnlyList<TimeOnly?>, DateXXArray : IReadOnlyList<DateOnly?>, IReadOnlyList<DateTime?>, BooleanArray : IReadOnlyList<bool?>, BinaryArray : IReadOnlyList<byte[]>.
The original issue also suggests implementing ICollection<T>.
Would you file followup work items so that this one can be marked as closed if checked-in as-is?
--
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
Re: [PR] GH-38348: [C#] Make PrimitiveArray support IReadOnlyList [arrow]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher merged PR #38680:
URL: https://github.com/apache/arrow/pull/38680
--
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