Open Veetaha opened 6 days ago
for compare with bon-generate all, if bon
only generate From<TBuilder> for T
use bon::bon;
// TODO: GENERATED By bon
impl<__KeepDim, __Mean> From<SumOptionalParamsBuilder<(__KeepDim, __Mean)>> for SumOptionalParams
where
__KeepDim: bon::private::IntoSet<Option<bool>, SumOptionalParamsBuilder__keep_dim>,
__Mean: bon::private::IntoSet<Option<bool>, SumOptionalParamsBuilder__mean>,
{
fn from(builder: SumOptionalParamsBuilder<(__KeepDim, __Mean)>) -> Self {
builder.build()
}
}
// User code
#[derive(bon::Builder)]
struct SumOptionalParams {
#[builder(default)]
keep_dim: bool,
#[builder(default)]
mean: bool,
}
struct Tensor {}
impl Tensor {
pub fn new() -> Self {
Tensor {}
}
// NOTE: orignal function need to be rewrite to accept options struct
fn _sum(&self, dims: &[i64], options: SumOptionalParams) -> Tensor {
Tensor {}
}
pub fn sum(&self, dims: &[i64]) -> Tensor {
let options = SumOptionalParams::builder().build();
self._sum(dims, options)
}
pub fn sum_with<T>(
&self,
dims: &[i64],
options: impl FnOnce(SumOptionalParamsBuilder) -> T,
) -> Tensor
where
T: Into<SumOptionalParams>,
{
let options = options(SumOptionalParams::builder()).into();
self._sum(dims, options)
}
}
fn main() {
let t = Tensor::new();
t.sum(&[1]).sum_with(&[1, 2], |b| b.keep_dim(true));
println!("test");
}
another version without closure, which allow early return and accept partial builder
use bon::bon;
// TODO: GENERATED By bon
impl<__KeepDim, __Mean> From<SumOptionalParamsBuilder<(__KeepDim, __Mean)>> for SumOptionalParams
where
__KeepDim: bon::private::IntoSet<Option<bool>, SumOptionalParamsBuilder__keep_dim>,
__Mean: bon::private::IntoSet<Option<bool>, SumOptionalParamsBuilder__mean>,
{
fn from(builder: SumOptionalParamsBuilder<(__KeepDim, __Mean)>) -> Self {
builder.build()
}
}
// User code
#[derive(bon::Builder)]
struct SumOptionalParams {
#[builder(default)]
keep_dim: bool,
#[builder(default)]
mean: bool,
}
struct Tensor {}
impl Tensor {
pub fn new() -> Self {
Tensor {}
}
// NOTE: orignal function need to be rewrite to accept options struct
fn _sum(&self, dims: &[i64], options: SumOptionalParams) -> Tensor {
Tensor {}
}
pub fn sum(&self, dims: &[i64]) -> Tensor {
let options = SumOptionalParams::builder().build();
self._sum(dims, options)
}
pub fn sum_with(&self, dims: &[i64], options: impl Into<SumOptionalParams>) -> Tensor {
let options = options.into();
self._sum(dims, options)
}
}
fn main() {
let t = Tensor::new();
let default_options = SumOptionalParams::builder().mean(true);
let output = t
.sum(&[1])
.sum_with(&[1, 2], SumOptionalParams::builder().keep_dim(true))
.sum_with(&[1, 2], default_options);
}
Yeah, I also was thinking of suggesting this, but this requires importing optional parameters struct type name.
another design is extract all params in function into param struct, like
use bon::bon;
// TODO: GENERATED By bon
impl<'a, __Dims, __KeepDim, __Mean> From<SumParamsBuilder<'a, (__Dims, __KeepDim, __Mean)>>
for SumParams<'a>
where
__Dims: bon::private::IntoSet<&'a [i64], SumParamsBuilder__dims>,
__KeepDim: bon::private::IntoSet<Option<bool>, SumParamsBuilder__keep_dim>,
__Mean: bon::private::IntoSet<Option<bool>, SumParamsBuilder__mean>,
{
fn from(builder: SumParamsBuilder<'a, (__Dims, __KeepDim, __Mean)>) -> Self {
builder.build()
}
}
#[derive(bon::Builder)]
#[builder(start_fn = new)]
struct SumParams<'a> {
dims: &'a [i64],
#[builder(default)]
keep_dim: bool,
#[builder(default)]
mean: bool,
}
struct Tensor {}
impl Tensor {
pub fn new() -> Self {
Tensor {}
}
pub fn sum<'b>(&self, params: impl Into<SumParams<'b>>) -> Tensor {
let params = params.into();
// todo logic
Tensor {}
}
}
// User can also implment into for SumParams
impl<'a, const N: usize> From<&'a [i64; N]> for SumParams<'a> {
fn from(dims: &'a [i64; N]) -> Self {
SumParams::new().dims(dims).build()
}
}
fn main() {
let t = Tensor::new();
let output = t
.sum(SumParams::new().dims(&[1, 2]))
.sum(SumParams::new().dims(&[1, 2]).keep_dim(false))
.sum(SumParams::new().dims(&[1, 2]).keep_dim(false).mean(false))
.sum(&[1, 2]);
//with https://github.com/elastio/bon/pull/125
//.sum(SumParams::new(&[1, 2]).keep_dim(false).mean(false));
}
good: only 1 variant of original function
bad: more verbose to call, can use custom implement into<FParams>
if only single required params.
not sure which style is better.
not sure if that was discussed, would be nice macro force all option parameters to be in the end in original function too(fail to compile if not). so macro will not reorder parameters as they written in text.
because if parameters will be reordered in macro, but not in original text, it may lead to subtle usage bugs.
Recap of above pattern. Personally, I prefer the closure style 1) > 2)
// 1)
let output = t
.sum(&[1]);
.sum_with(&[1, 2], |b| b.keep_dim(true).mean(false));
// 2)
let output = t
.sum(|b| b.dims(&[1, 2]))
.sum(|b| b.dims(&[1, 2]).keep_dim(false).mean(false));
// 3)
let output = t
.sum(&[1])
.sum_with(&[1, 2], TensorSum__OptParams::builder().keep_dim(true).mean(false));
// 4)
let output = t
.sum(TensorSum__Params::new().dims(&[1, 2]))
.sum(TensorSum__Params::new().dims(&[1, 2]).keep_dim(false).mean(false));
using closure
capture only optional function params to param struct
capture all function params to param struct
This is based on a comment from @cksac in https://github.com/elastio/bon/issues/85#issuecomment-2343780384 It should look something like this. An additional config should enable the generation of a separate syntax for the builder where all required parameters are passed as positional parameters at the start of the function and optional parameters configured with the builder passed as an input to the trailing closure argument.
I think this API looks too exotic, and it's probably fine if it would require a bit more code to write for the user. For example, if we just generate a
From<TBuilder> for T
impl, this API can be written manually using existingbon
primitives (see the comment https://github.com/elastio/bon/issues/85#issuecomment-2343850854).A note for the community from the maintainers
Please vote on this issue by adding a 👍 reaction to help the maintainers with prioritizing it. You may add a comment describing your real use case related to this issue for us to better understand the problem domain.