Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Added iterator for StructArray #613

Conversation

illumination-k
Copy link
Contributor

I implemented Iterator for StructArray.
Also, the struct_.rs file was changed to a directory with mod.rs and the ffi part was split into ffi.rs.

If you need me to open an issue on this PR, I will do it.

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #613 (ba1ca05) into main (9d4107c) will decrease coverage by 0.03%.
The diff coverage is 67.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #613      +/-   ##
==========================================
- Coverage   79.61%   79.58%   -0.04%     
==========================================
  Files         403      406       +3     
  Lines       24838    24882      +44     
==========================================
+ Hits        19775    19802      +27     
- Misses       5063     5080      +17     
Impacted Files Coverage Δ
src/array/struct_/mod.rs 56.92% <ø> (ø)
tests/it/array/mod.rs 100.00% <ø> (ø)
src/array/struct_/iterator.rs 40.00% <40.00%> (ø)
src/array/struct_/ffi.rs 88.88% <88.88%> (ø)
tests/it/array/struct_/iterator.rs 100.00% <100.00%> (ø)
src/compute/arithmetics/time.rs 46.93% <0.00%> (+2.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d4107c...ba1ca05. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Thanks for the PR! No need for an issue :)

The iterators of arrays are iterators over rows.

An iterator over a StructArray would be something like Iterator<Item=Option<Vec<Box<dyn Scalar>>>, since each row is composed by multiple entries (one per field) where the Option is the Struct's validity.

@illumination-k
Copy link
Contributor Author

Thanks for your comments! I understood it wrong. Maybe, I think I have corrected it.

Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, @illumination-k, Looks great!

I left a comment to simplify the loop.

The idea in Rust is that when using iterator pattern, rust skips bound checks (it also makes it a bit easier to read for non-C developers)

src/array/struct_/iterator.rs Outdated Show resolved Hide resolved
src/array/struct_/iterator.rs Outdated Show resolved Hide resolved
@jorgecarleitao jorgecarleitao merged commit b3ed162 into jorgecarleitao:main Nov 19, 2021
@jorgecarleitao jorgecarleitao added enhancement An improvement to an existing feature feature A new feature and removed enhancement An improvement to an existing feature labels Nov 19, 2021
@jorgecarleitao jorgecarleitao changed the title Implement iterator for StructArray Added iterator for StructArray Nov 19, 2021
@illumination-k illumination-k deleted the feature/impl-iterator-structarray branch November 19, 2021 20:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants