Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement From<Array<A, D>> for ArcArray<A, D> #1021

Merged
merged 1 commit into from Jun 5, 2021

Conversation

jturner314
Copy link
Member

I'd like to make CowRepr implement DataOwned, where DataOwned is changed to mean "a storage type which can own its data". The only issue is the .into_shared() method, which says, "Turn the array into a shared ownership (copy on write) array, without any copying." We can't always turn a CowArray into an ArcArray without copying. One approach (which would be a breaking change) would be to remove DataOwned::into_shared and just rely on this From implementation instead. Then, we could implement DataOwned for CowRepr without issues.

Regardless of the CowRepr stuff, this impl still seems useful.

@bluss bluss added this to the 0.15.3 milestone Jun 5, 2021
@bluss
Copy link
Member

bluss commented Jun 5, 2021

Agree. I hope to keep the into_shared etc methods somehow, but it's clear that they need to change to support wider diversity of owned arrays (including future differences in allocator). Maybe adding a Clone bound to into_shared() would be ok and sufficient, and then the specific From impls can be used by users when they need to avoid Clone requirements.

@bluss bluss merged commit f5c18a5 into rust-ndarray:master Jun 5, 2021
@jturner314
Copy link
Member Author

Maybe adding a Clone bound to into_shared() would be ok and sufficient, and then the specific From impls can be used by users when they need to avoid Clone requirements.

Yeah, I think this makes the most sense. We'd move into_shared from DataOwned to Data, add an A: Clone bound, and drop the "without any copying" guarantee. In most cases, the user would just use .into_shared() to convert arrays to ArcArray (so that all they'd need is are A: Clone and S: Data bounds), but if they want to create an ArcArray from an array without A: Clone, they would use an Into<ArcArray> bound instead.

@jturner314 jturner314 deleted the impl-from-Array-for-ArcArray branch June 5, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants