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

Vortex Error #133

Merged
merged 4 commits into from
Mar 25, 2024
Merged

Vortex Error #133

merged 4 commits into from
Mar 25, 2024

Conversation

gatesn
Copy link
Contributor

@gatesn gatesn commented Mar 25, 2024

We should do more work on our error crate, taking some inspiration from Polars.

@robert3005 robert3005 enabled auto-merge (squash) March 25, 2024 11:07
@robert3005
Copy link
Member

Let's say this fixes #125

@robert3005 robert3005 merged commit 8668b3b into develop Mar 25, 2024
2 checks passed
@robert3005 robert3005 deleted the ngates/vortex-error branch March 25, 2024 11:08
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());
Copy link
Member

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()),
Copy link
Member

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,
Copy link
Member

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());
Copy link
Member

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()),
Copy link
Member

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()),
Copy link
Member

Choose a reason for hiding this comment

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

but got...?

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

Successfully merging this pull request may close these issues.

3 participants