feat: add location for all error #1475
Conversation
|
ACTION NEEDED Lance follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
Weijun-H
changed the title
Weijun-H
force-pushed
the
location-all-error
branch
2 times, most recently
from
1edf686
to
92a89f7
Compare
eddyxu
requested review from
westonpace and
wjones127
and removed request for
westonpace
rust/lance/src/index/vector/ivf.rs
Outdated
| @@ -573,14 +577,15 @@ fn sanity_check<'a>(dataset: &'a Dataset, column: &str) -> Result<&'a Field> { | |||
| return Err( | |||
| Error::Index{message: | |||
| format!("VectorIndex requires the column data type to be fixed size list of float32s, got {}", | |||
| elem_type.data_type())}); | |||
| elem_type.data_type()), location: location!() }); | |||
There was a problem hiding this comment.
The formatting on this line seems slightly off but maybe it's just github
| .map_err(|err| crate::Error::PrerequisiteFailed { message: err }) | ||
| .map_err(|err| crate::Error::PrerequisiteFailed { | ||
| message: err, | ||
| location: location!(), |
There was a problem hiding this comment.
Can we reuse the inner error's location here?
| .map_err(|err| crate::Error::PrerequisiteFailed { message: err }) | ||
| .map_err(|err| crate::Error::PrerequisiteFailed { | ||
| message: err, | ||
| location: location!(), |
There was a problem hiding this comment.
Can we reuse the inner error's location here?
There was a problem hiding this comment.
It is hard to reuse location here because it is hard to add Clone for Error
| @@ -87,6 +104,7 @@ impl From<ArrowError> for Error { | |||
| fn from(e: ArrowError) -> Self { | |||
| Self::Arrow { | |||
| message: e.to_string(), | |||
| location: location!(), | |||
There was a problem hiding this comment.
This location is unlikely to be very useful but I'm not sure we can do much better since From doesn't give us the option of forwarding the location.
Weijun-H
force-pushed
the
location-all-error
branch
2 times, most recently
from
c4afebe
to
a65b895
Compare
Weijun-H
force-pushed
the
location-all-error
branch
from
4687cd3
to
afff760
Compare
Closed #1275