-
Notifications
You must be signed in to change notification settings - Fork 33
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
Vortex Error #133
Vortex Error #133
Conversation
Let's say this fixes #125 |
use crate::scalar::Scalar; | ||
|
||
// TODO(ngates): convert this to arithmetic operations with macro over the kernel. | ||
pub fn add(lhs: &dyn Array, rhs: &dyn Array) -> VortexResult<ArrayRef> { | ||
// Check that the arrays are the same length. | ||
let length = lhs.len(); | ||
if rhs.len() != length { | ||
return Err(VortexError::LengthMismatch); | ||
return Err("Arrays have different lengths".into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format in the lengths?
@@ -51,7 +51,7 @@ impl ALPArray { | |||
pub fn encode(array: &dyn Array) -> VortexResult<ArrayRef> { | |||
match ArrayKind::from(array) { | |||
ArrayKind::Primitive(p) => Ok(alp_encode(p)?.into_array()), | |||
_ => Err(VortexError::InvalidEncoding(array.encoding().id())), | |||
_ => Err("ALP can only encoding primitive arrays".into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/encode/encoding/
also might be nice to propagate information about what encoding was passed in?
@@ -15,8 +16,8 @@ impl PatchFn for PrimitiveArray { | |||
// TODO(ngates): support a default implementation based on iter_arrow? | |||
_ => Err(VortexError::MissingKernel( | |||
"patch", | |||
self.encoding().id(), | |||
vec![patch.encoding().id()], | |||
self.encoding().id().0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.name() instead of .0
@@ -34,7 +34,7 @@ impl RoaringIntArray { | |||
|
|||
pub fn try_new(bitmap: Bitmap, ptype: PType) -> VortexResult<Self> { | |||
if !ptype.is_unsigned_int() { | |||
return Err(VortexError::InvalidPType(ptype)); | |||
return Err("RoaringInt expected unsigned int".into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... "expected unsigned int ptype" and then inline what was received
@@ -55,7 +55,7 @@ impl RoaringIntArray { | |||
pub fn encode(array: &dyn Array) -> VortexResult<Self> { | |||
match ArrayKind::from(array) { | |||
ArrayKind::Primitive(p) => Ok(roaring_encode(p)), | |||
_ => Err(VortexError::InvalidEncoding(array.encoding().id())), | |||
_ => Err("RoaringInt can only encode primitive arrays".into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but got...?
@@ -40,7 +40,7 @@ impl ZigZagArray { | |||
pub fn encode(array: &dyn Array) -> VortexResult<ArrayRef> { | |||
match ArrayKind::from(array) { | |||
ArrayKind::Primitive(p) => Ok(zigzag_encode(p)?.into_array()), | |||
_ => Err(VortexError::InvalidEncoding(array.encoding().id())), | |||
_ => Err("ZigZag can only encoding primitive arrays".into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but got...?
We should do more work on our error crate, taking some inspiration from Polars.