-
Notifications
You must be signed in to change notification settings - Fork 689
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
Add .arc()
method for types that are typically used as *Refs
#5714
Comments
I believe you can already use |
That works with SchemaRef (thanks!) but not ArrayRef, because ArrayRef is
I believe this is because Arc's generic Would it be okay if I made a PR on PrimitiveArray to give it an arc or to_arc method? |
I'm honestly not sure, it feels a little bit niche imo, to avoid one fairly common import... FWIW methods that don't require ownership should take |
The above example is from More than avoiding an import it's about readability IMO. When reading code I think seeing the Arc::new is a distraction - hiding those sorts of wrapper types at the end with a |
I think lets see what other maintainers think, I'm not sure about adding an additional member function to every array type just for this. Perhaps you could use an extension trait in your codebase for this? |
Sure, it's possible to write something like
and impl it for the array types. I took a quick look and it seems like there's 17 structs if we were to implement it directly.
Although probably implementing it for ArrayRef is bad. Additionally, |
I think I agree with @tustvold here. I would be someone that theoretically would benefit from this but have never missed it. Also, there is a solution via the helper trait. Just my 2 cents. |
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
Disclaimer: I only started using arrow-rs the other day via lancedb.
It seems annoying to have to both remember to import Arc and then write out
Arc::new(..)
every time you want to use a FooRef type.Describe the solution you'd like
Wouldn't it make more sense to implement a
fn arc(self) -> Arc<Self> { Arc::new(self) }
on types for which FooRef is commonly used?Describe alternatives you've considered
Maybe I've just been looking at a lot of example code and typically you load schemas/arrays from things that already return them wrapped in Arc?
Also,
to_arc
is maybe more in line with Rust naming guidelines. I don't thinkto_ref
orref
is the right choice since it's not returning a &T.The text was updated successfully, but these errors were encountered: