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

Change signature of PrimitiveScalar::value to return reference #1129

Merged

Conversation

ncpenke
Copy link
Contributor

@ncpenke ncpenke commented Jun 29, 2022

The use-case is to be able to write generic code that uses scalars with Array iterators (for example in arrow2-convert for the deserialization path). This is much easier if the PrimitiveScalar::value() is consistent with the rest of scalars and returns a reference.

This could be too high risk of a change if too many downstream users will be affected, so alternately a new method PrimitiveScalar::value_ref() could be introduces that returns the reference. But that seemed less clean, so opted for this change to get feedback. There seems to be good test coverage around scalars, and all the tests are passing.

@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #1129 (b8759df) into main (81ab424) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #1129      +/-   ##
==========================================
- Coverage   83.37%   83.37%   -0.01%     
==========================================
  Files         367      367              
  Lines       35574    35574              
==========================================
- Hits        29661    29659       -2     
- Misses       5913     5915       +2     
Impacted Files Coverage Δ
src/compute/arithmetics/decimal/mul.rs 75.77% <0.00%> (ø)
src/compute/arithmetics/time.rs 72.19% <50.00%> (ø)
src/compute/arithmetics/decimal/div.rs 95.39% <100.00%> (ø)
src/compute/arithmetics/mod.rs 74.10% <100.00%> (ø)
src/scalar/primitive.rs 80.00% <100.00%> (ø)
src/array/binary/mod.rs 88.88% <0.00%> (-1.24%) ⬇️
src/io/ipc/read/reader.rs 95.66% <0.00%> (-0.37%) ⬇️
src/bitmap/immutable.rs 81.48% <0.00%> (+0.61%) ⬆️
src/bitmap/utils/slice_iterator.rs 98.78% <0.00%> (+1.21%) ⬆️

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 81ab424...b8759df. Read the comment docs.

@jorgecarleitao jorgecarleitao merged commit a080e1a into jorgecarleitao:main Jul 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants