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

Adding new constructor for MutableListArray #1503

Merged

Conversation

cjermain
Copy link
Contributor

@cjermain cjermain commented Jun 8, 2023

This PR addresses #1502 by introducing a new constructor for MutableListArray that allows for building an object based on existing values from a MutableArray, Offsets, and MutableBitmap. I'm interested in feedback on this relative to the question posed in #1502, and happy to add tests if this seems like the right approach.

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.15 ⚠️

Comparison is base (99e30d3) 83.58% compared to head (3557894) 83.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1503      +/-   ##
==========================================
- Coverage   83.58%   83.43%   -0.15%     
==========================================
  Files         388      388              
  Lines       41912    42010      +98     
==========================================
+ Hits        35034    35053      +19     
- Misses       6878     6957      +79     
Impacted Files Coverage Δ
src/array/list/mutable.rs 74.24% <0.00%> (-5.65%) ⬇️

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sundy-li sundy-li merged commit 0d568a3 into jorgecarleitao:main Jun 19, 2023
validity: Option<MutableBitmap>,
) -> Self {
assert_eq!(values.len(), offsets.last().to_usize());
let data_type = values.data_type().clone();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sundy-li, thanks for the quick response. Unfortunately I missed on the dtype -- it should be the following:

        let data_type = ListArray::<O>::default_datatype(values.data_type().clone());

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants