Conversation
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
|
Should we deprecate the old API and add this new one or just break it? |
Merging this PR will not alter performance
|
Breaking doesn't sound too bad to me, wdyt? |
…to bp/arrow-with-sesh
palaska
added
changelog/deprecation
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
| pub trait FromArrowArray<A>: Sized { | ||
| /// Convert an Arrow array to a Vortex array, looking up extension types in `session`. | ||
| fn from_arrow_with_session( | ||
| array: A, | ||
| nullable: bool, | ||
| session: &VortexSession, | ||
| ) -> VortexResult<Self>; | ||
|
|
||
| /// Convert an Arrow array to a Vortex array using the legacy global session. | ||
| #[deprecated(note = "Use `from_arrow_with_session` instead")] | ||
| fn from_arrow(array: A, nullable: bool) -> VortexResult<Self> { | ||
| Self::from_arrow_with_session(array, nullable, &LEGACY_SESSION) | ||
| } |
There was a problem hiding this comment.
So the plan is the release this and then later rename from_arrow_with_session -> from_arrow (with session) once after removing the deprecated from_arrow (no session)?
There was a problem hiding this comment.
I think so, or we can find a better name (maybe from_arrow_in?) and just delete the old function?
3 participants
Summary
This PR is a mechanical refactor that deprecates
from_arrowin favor offrom_arrow_in, that accepts a Vortex session argument. This will be needed for arrow roundtrip with customized sessions with extension types.This is a prerequisite for #7726
API Changes
from_arrowis deprecated.from_arrow_inshould be used and it requires VortexSession to be passed.