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

Added to conversion to FixedSizeBinary #622

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

ritchie46
Copy link
Collaborator

This adds fn to conversion for FixedSizeBinary. This makes converting to another logical type consistent with primitive arrays.

I could add it to the other arrays as well.

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #622 (ec52134) into main (61c2f3c) will increase coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #622      +/-   ##
==========================================
+ Coverage   69.71%   69.74%   +0.03%     
==========================================
  Files         299      299              
  Lines       16599    16677      +78     
==========================================
+ Hits        11572    11632      +60     
- Misses       5027     5045      +18     
Impacted Files Coverage Δ
src/array/fixed_size_binary/mod.rs 69.62% <0.00%> (-10.09%) ⬇️
src/bitmap/utils/slice_iterator.rs 91.04% <0.00%> (-1.50%) ⬇️
src/array/union/ffi.rs 0.00% <0.00%> (ø)
src/temporal_conversions.rs 74.82% <0.00%> (+0.34%) ⬆️
src/io/parquet/read/nested_utils.rs 78.43% <0.00%> (+0.98%) ⬆️
src/io/csv/write/serialize.rs 49.13% <0.00%> (+15.19%) ⬆️

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 61c2f3c...ec52134. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Thanks! I think we need to check that the data_type is a valid one and panic if not, like we do in from_data.

@ritchie46
Copy link
Collaborator Author

Thanks! I think we need to check that the data_type is a valid one and panic if not, like we do in from_data.

I didn't fully understand what we mean by valid in this case? If we use an extension type, do we know what is valid or not?

@jorgecarleitao
Copy link
Owner

jorgecarleitao commented Nov 22, 2021

The function get_size is the one that validates in the from_data, here:

        match data_type.to_logical_type() {
            DataType::FixedSizeBinary(size) => *size,
            _ => panic!("Wrong DataType"),
        }

in this case we need to verify that the size is still the same.

@jorgecarleitao
Copy link
Owner

(note that the .to_logical_type converts any extension type to its internal type, to take extension types into account ^_^)

/// Panics iff the data_type is not supported for the physical type.
#[inline]
pub fn to(self, data_type: DataType) -> Self {
match (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Something like this? I assume we must check that the physical types are correct.

Copy link
Owner

Choose a reason for hiding this comment

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

exactly, this works!

@jorgecarleitao jorgecarleitao merged commit bc3da78 into jorgecarleitao:main Nov 24, 2021
@jorgecarleitao jorgecarleitao changed the title add to conversion to fixedsizebinary Added to conversion to FixedSizeBinary Nov 24, 2021
@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants