Consider using traits instead of `NumericSexp` and `NumericScalar`

[daniellga]

I saw you added these 2 types which are structs that hold other Sexp types. This issue is just a suggestion of what I think would be a cleaner and more mantainable code.

So instead of using for example NumericScalar struct, I think you could use a Scalar trait.

trait Scalar<T> {
    fn as_scalar(self) -> savvy::Result<T>;
}

impl Scalar<i32> for Sexp {
    fn as_scalar(self) -> savvy::Result<i32> {
        match self.into_typed() {
            TypedSexp::Integer(integer_sexp) if integer_sexp.len() == 1 => {
                Ok(integer_sexp.as_slice()[0])
            }
            _ => {
                let err = "Argument must be an integer of length 1.".to_string();
                Err(err.into())
            }
        }
    }
}

impl Scalar<f64> for Sexp {
    fn as_scalar(self) -> savvy::Result<f64> {
        match self.into_typed() {
            TypedSexp::Real(real_sexp) if real_sexp.len() == 1 => Ok(real_sexp.as_slice()[0]),
            _ => {
                let err = "Argument must be a double of length 1.".to_string();
                Err(err.into())
            }
        }
    }
}

impl Scalar<bool> for Sexp {
    fn as_scalar(self) -> savvy::Result<bool> {
        match self.into_typed() {
            TypedSexp::Logical(logical_sexp) if logical_sexp.len() == 1 => {
                Ok(logical_sexp.as_slice_raw()[0] == 1)
            }
            _ => {
                let err = "Argument must be a logical of length 1.".to_string();
                Err(err.into())
            }
        }
    }
}

And I think the same could be done for NumericSexp.

So instead of making the user use these structs as types in the code, they would do the following when needed to retrieve a scalar from R:

   fn fff(a: Sexp) -> savvy::Result<()> {
        let a: i32 = a.as_scalar()?;
        ...
   }

This is just a suggestion. Let me know if you like it so I can make a draft to show you.

[yutannihilation]

Yeah, I know it's a choice and what the majority of people think the Rusty way. But, on the other hand, it makes it extremely difficult to do static analysis and proc macro, which are the core parts of Savvy. So, I think I will never implement this unless some feature is provided by Rust.

[daniellga]

Could you elaborate a little further on this static analysis and proc macro stuff? I am having a hard time understanding how having a static type is advantageous here since you implement for these new types basically the same functions that you already have in Sexp.

[yutannihilation]

Sorry for replying late. I was thinking if I will try implementing this.

The main difficulty is this part. Sexp means savvy can know nothing about the user's assumption on the type. This doesn't matter much at the moment, but I want to keep the possibility of distinguishing the input types in case I might want to add some type-specific codes (e.g. add some vctrs function in the R wrapper's side for better validation). So, let me decline your suggestion. Sorry.

   fn fff(a: Sexp)
             ^^^^