Spanned Value step 1: span all value cases (#10042)

# Description

This doesn't really do much that the user could see, but it helps get us
ready to do the steps of the refactor to split the span off of Value, so
that values can be spanless. This allows us to have top-level values
that can hold both a Value and a Span, without requiring that all values
have them.

We expect to see significant memory reduction by removing so many
unnecessary spans from values. For example, a table of 100,000 rows and
5 columns would have a savings of ~8megs in just spans that are almost
always duplicated.

# User-Facing Changes

Nothing yet

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
JT
2023-08-25 08:48:05 +12:00
committed by GitHub
parent 8da27a1a09
commit 1e3e034021
234 changed files with 1153 additions and 868 deletions

View File

@ -92,7 +92,7 @@ fn command_eager(
df: NuDataFrame,
) -> Result<PipelineData, ShellError> {
let mask_value: Value = call.req(engine_state, stack, 0)?;
let mask_span = mask_value.span()?;
let mask_span = mask_value.span();
if NuExpression::can_downcast(&mask_value) {
let expression = NuExpression::try_from_value(mask_value)?;

View File

@ -160,7 +160,7 @@ fn command_lazy(
let value: Value = call.req(engine_state, stack, 1)?;
return Err(ShellError::IncompatibleParametersSingle {
msg: "New name list has different size to column list".into(),
span: value.span()?,
span: value.span(),
});
}

View File

@ -134,16 +134,17 @@ fn command(
))
}
}
_ => match value.span() {
Ok(span) => Err(ShellError::GenericError(
Value::Error { error, .. } => Err(*error.clone()),
_ => {
let span = value.span();
Err(ShellError::GenericError(
"Incorrect value for quantile".to_string(),
"value should be a float".to_string(),
Some(span),
None,
Vec::new(),
)),
Err(e) => Err(e),
},
))
}
})
.collect::<Result<Vec<f64>, ShellError>>()
});

View File

@ -93,7 +93,7 @@ fn command(
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let index_value: Value = call.req(engine_state, stack, 0)?;
let index_span = index_value.span()?;
let index_span = index_value.span();
let index = NuDataFrame::try_from_value(index_value)?.as_series(index_span)?;
let casted = match index.dtype() {

View File

@ -114,7 +114,7 @@ impl Command for WithColumn {
Err(ShellError::CantConvert {
to_type: "lazy or eager dataframe".into(),
from_type: value.get_type().to_string(),
span: value.span()?,
span: value.span(),
help: None,
})
}
@ -128,7 +128,7 @@ fn command_eager(
mut df: NuDataFrame,
) -> Result<PipelineData, ShellError> {
let new_column: Value = call.req(engine_state, stack, 0)?;
let column_span = new_column.span()?;
let column_span = new_column.span();
if NuExpression::can_downcast(&new_column) {
let vals: Vec<Value> = call.rest(engine_state, stack, 0)?;

View File

@ -93,7 +93,7 @@ impl Command for LazyFillNA {
None,
))
} else {
let val_span = value.span()?;
let val_span = value.span();
let frame = NuDataFrame::try_from_value(value)?;
let columns = frame.columns(val_span)?;
let dataframe = columns

View File

@ -126,7 +126,7 @@ impl Command for ToLazyGroupBy {
let value: Value = call.req(engine_state, stack, 0)?;
return Err(ShellError::IncompatibleParametersSingle {
msg: "Expected only Col expressions".into(),
span: value.span()?,
span: value.span(),
});
}

View File

@ -199,7 +199,7 @@ impl Command for LazyJoin {
let right_on: Value = call.req(engine_state, stack, 2)?;
return Err(ShellError::IncompatibleParametersSingle {
msg: "The right column list has a different size to the left column list".into(),
span: right_on.span()?,
span: right_on.span(),
});
}
@ -209,7 +209,7 @@ impl Command for LazyJoin {
let value: Value = call.req(engine_state, stack, *index)?;
return Err(ShellError::IncompatibleParametersSingle {
msg: "Expected only a string, col expressions or list of strings".into(),
span: value.span()?,
span: value.span(),
});
}
}

View File

@ -118,7 +118,7 @@ impl Command for LazySortBy {
let span = call
.get_flag::<Value>(engine_state, stack, "reverse")?
.expect("already checked and it exists")
.span()?;
.span();
return Err(ShellError::GenericError(
"Incorrect list size".into(),
"Size doesn't match expression list".into(),

View File

@ -82,7 +82,7 @@ fn command(
let indices_value: Value = call
.get_flag(engine_state, stack, "indices")?
.expect("required named value");
let indices_span = indices_value.span()?;
let indices_span = indices_value.span();
let indices = NuDataFrame::try_from_value(indices_value)?.as_series(indices_span)?;
let casted = match indices.dtype() {
@ -204,7 +204,7 @@ fn command(
"this value cannot be set in a series of type '{}'",
series.dtype()
),
Some(value.span()?),
Some(value.span()),
None,
Vec::new(),
)),

View File

@ -74,7 +74,7 @@ fn command(
let df = NuDataFrame::try_from_pipeline(input, call.head)?;
let other_value: Value = call.req(engine_state, stack, 0)?;
let other_span = other_value.span()?;
let other_span = other_value.span();
let other_df = NuDataFrame::try_from_value(other_value)?;
let other = other_df.as_series(other_span)?;

View File

@ -81,7 +81,7 @@ fn command(
let mask_value: Value = call
.get_flag(engine_state, stack, "mask")?
.expect("required named value");
let mask_span = mask_value.span()?;
let mask_span = mask_value.span();
let mask = NuDataFrame::try_from_value(mask_value)?.as_series(mask_span)?;
let bool_mask = match mask.dtype() {
@ -185,7 +185,7 @@ fn command(
"this value cannot be set in a series of type '{}'",
series.dtype()
),
Some(value.span()?),
Some(value.span()),
None,
Vec::new(),
)),

View File

@ -74,7 +74,7 @@ fn command(
let df = NuDataFrame::try_from_pipeline(input, call.head)?;
let other: Value = call.req(engine_state, stack, 0)?;
let other_span = other.span()?;
let other_span = other.span();
let other_df = NuDataFrame::try_from_value(other)?;
let other_series = other_df.as_series(other_span)?;

View File

@ -9,7 +9,7 @@ pub fn extract_strings(value: Value) -> Result<Vec<String>, ShellError> {
(Err(_), Ok(cols)) => Ok(cols),
_ => Err(ShellError::IncompatibleParametersSingle {
msg: "Expected a string or list of strings".into(),
span: value.span()?,
span: value.span(),
}),
}
}

View File

@ -17,7 +17,7 @@ pub(super) fn between_dataframes(
right: &Value,
rhs: &NuDataFrame,
) -> Result<Value, ShellError> {
let operation_span = span(&[left.span()?, right.span()?]);
let operation_span = span(&[left.span(), right.span()]);
match operator.item {
Operator::Math(Math::Plus) => match lhs.append_df(rhs, Axis::Row, operation_span) {
Ok(df) => Ok(df.into_value(operation_span)),
@ -26,9 +26,9 @@ pub(super) fn between_dataframes(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
}
}
@ -40,7 +40,7 @@ pub(super) fn compute_between_series(
right: &Value,
rhs: &Series,
) -> Result<Value, ShellError> {
let operation_span = span(&[left.span()?, right.span()?]);
let operation_span = span(&[left.span(), right.span()]);
match operator.item {
Operator::Math(Math::Plus) => {
let mut res = lhs + rhs;
@ -71,7 +71,7 @@ pub(super) fn compute_between_series(
Err(e) => Err(ShellError::GenericError(
"Division error".into(),
e.to_string(),
Some(right.span()?),
Some(right.span()),
None,
Vec::new(),
)),
@ -79,32 +79,32 @@ pub(super) fn compute_between_series(
}
Operator::Comparison(Comparison::Equal) => {
let name = format!("eq_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Comparison(Comparison::NotEqual) => {
let name = format!("neq_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Comparison(Comparison::LessThan) => {
let name = format!("lt_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Comparison(Comparison::LessThanOrEqual) => {
let name = format!("lte_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Comparison(Comparison::GreaterThan) => {
let name = format!("gt_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Comparison(Comparison::GreaterThanOrEqual) => {
let name = format!("gte_{}_{}", lhs.name(), rhs.name());
let res = compare_series(lhs, rhs, name.as_str(), right.span().ok(), Series::equal)?;
let res = compare_series(lhs, rhs, name.as_str(), right.span(), Series::equal)?;
NuDataFrame::series_to_value(res, operation_span)
}
Operator::Boolean(Boolean::And) => match lhs.dtype() {
@ -122,7 +122,7 @@ pub(super) fn compute_between_series(
_ => Err(ShellError::GenericError(
"Incompatible types".into(),
"unable to cast to boolean".into(),
Some(right.span()?),
Some(right.span()),
None,
Vec::new(),
)),
@ -151,7 +151,7 @@ pub(super) fn compute_between_series(
_ => Err(ShellError::GenericError(
"Incompatible types".into(),
"unable to cast to boolean".into(),
Some(right.span()?),
Some(right.span()),
None,
Vec::new(),
)),
@ -168,9 +168,9 @@ pub(super) fn compute_between_series(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
}
}
@ -179,7 +179,7 @@ fn compare_series<'s, F>(
lhs: &'s Series,
rhs: &'s Series,
name: &'s str,
span: Option<Span>,
span: Span,
f: F,
) -> Result<Series, ShellError>
where
@ -190,7 +190,7 @@ where
ShellError::GenericError(
"Equality error".into(),
e.to_string(),
span,
Some(span),
None,
Vec::new(),
)
@ -211,13 +211,13 @@ pub(super) fn compute_series_single_value(
return Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
});
}
let lhs_span = left.span()?;
let lhs_span = left.span();
let lhs = lhs.as_series(lhs_span)?;
match operator.item {
@ -232,9 +232,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Math(Math::Minus) => match &right {
@ -247,9 +247,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Math(Math::Multiply) => match &right {
@ -262,9 +262,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Math(Math::Divide) => match &right {
@ -285,9 +285,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::Equal) => match &right {
@ -305,9 +305,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::NotEqual) => match &right {
@ -326,9 +326,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::LessThan) => match &right {
@ -342,9 +342,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::LessThanOrEqual) => match &right {
@ -358,9 +358,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::GreaterThan) => match &right {
@ -374,9 +374,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::GreaterThanOrEqual) => match &right {
@ -390,9 +390,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
// TODO: update this to do a regex match instead of a simple contains?
@ -401,9 +401,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::StartsWith) => match &right {
@ -414,9 +414,9 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
Operator::Comparison(Comparison::EndsWith) => match &right {
@ -427,17 +427,17 @@ pub(super) fn compute_series_single_value(
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
},
_ => Err(ShellError::OperatorMismatch {
op_span: operator.span,
lhs_ty: left.get_type().to_string(),
lhs_span: left.span()?,
lhs_span: left.span(),
rhs_ty: right.get_type().to_string(),
rhs_span: right.span()?,
rhs_span: right.span(),
}),
}
}

View File

@ -882,6 +882,7 @@ fn series_to_values(
span,
Span::unknown(),
)),
span,
}
}
};
@ -897,6 +898,7 @@ fn series_to_values(
span,
Span::unknown(),
)),
span,
}
}
};
@ -950,6 +952,7 @@ fn series_to_values(
span,
Span::unknown(),
)),
span,
}
}
};
@ -965,6 +968,7 @@ fn series_to_values(
span,
Span::unknown(),
)),
span,
}
}
};

View File

@ -235,7 +235,7 @@ impl NuDataFrame {
if Self::can_downcast(&value) {
Ok(Self::get_df(value)?)
} else if NuLazyFrame::can_downcast(&value) {
let span = value.span()?;
let span = value.span();
let lazy = NuLazyFrame::try_from_value(value)?;
let df = lazy.collect(span)?;
Ok(df)
@ -243,7 +243,7 @@ impl NuDataFrame {
Err(ShellError::CantConvert {
to_type: "lazy or eager dataframe".into(),
from_type: value.get_type().to_string(),
span: value.span()?,
span: value.span(),
help: None,
})
}
@ -266,7 +266,7 @@ impl NuDataFrame {
x => Err(ShellError::CantConvert {
to_type: "dataframe".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}

View File

@ -70,17 +70,17 @@ fn compute_with_value(
match rhs.as_ref() {
polars::prelude::Expr::Literal(..) => {
with_operator(operator, left, rhs, lhs_span, right.span()?, op)
with_operator(operator, left, rhs, lhs_span, right.span(), op)
}
_ => Err(ShellError::TypeMismatch {
err_message: "Only literal expressions or number".into(),
span: right.span()?,
span: right.span(),
}),
}
}
_ => {
let rhs = NuExpression::try_from_value(right.clone())?;
with_operator(operator, left, &rhs, lhs_span, right.span()?, op)
with_operator(operator, left, &rhs, lhs_span, right.span(), op)
}
}
}

View File

@ -79,7 +79,7 @@ impl NuExpression {
x => Err(ShellError::CantConvert {
to_type: "lazy expression".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}
@ -157,7 +157,7 @@ impl ExtractedExpr {
x => Err(ShellError::CantConvert {
to_type: "expression".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}

View File

@ -135,7 +135,7 @@ impl NuLazyFrame {
Err(ShellError::CantConvert {
to_type: "lazy or eager dataframe".into(),
from_type: value.get_type().to_string(),
span: value.span()?,
span: value.span(),
help: None,
})
}
@ -164,7 +164,7 @@ impl NuLazyFrame {
x => Err(ShellError::CantConvert {
to_type: "lazy frame".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}

View File

@ -104,7 +104,7 @@ impl NuLazyGroupBy {
x => Err(ShellError::CantConvert {
to_type: "lazy groupby".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}

View File

@ -71,7 +71,7 @@ impl NuWhen {
x => Err(ShellError::CantConvert {
to_type: "when expression".into(),
from_type: x.get_type().to_string(),
span: x.span()?,
span: x.span(),
help: None,
}),
}

View File

@ -21,7 +21,7 @@ pub(crate) fn convert_columns(
Vec::new(),
)
})
.and_then(|v| v.span())?;
.map(|v| v.span())?;
let res = columns
.into_iter()
@ -61,7 +61,7 @@ pub(crate) fn convert_columns_string(
Vec::new(),
)
})
.and_then(|v| v.span())?;
.map(|v| v.span())?;
let res = columns
.into_iter()