@dawnandrew100 , I implemented pratt parsing for now, yet I believe this doesn't comply with the architecture of the project, I'll be trying to fit this in as you suggest.

also, what sort of test do you expect?

@shri-acha
As far as testing for the parser, I think it should handle a number of basic cases, errors, and any edge case you come across.

So, some things that I think should be tested for are:

• Single number [4]
• Single variable [x]
• Exponents [x^2]
• Integer plus variable [4x]
• Float plus variable [4.2x]
• Basic expression [4x + 2]
• Ints and floats with exponents [4x^2 + 2.3x^3]
• Other operators [4x + 2 - 5x^2 * 4x^4 / 6x^6]
• Zero x [0x] <- This and any power of x can just be evaluated to 0
• Zero [0]
• Multivariate expressions [4xy + 4x^2 - 2y + 4]
• Invalid expressions [ 4 +++ 3x]
• Missing right hand [4x +]
• Missing left hand [ + 3x]
• Only operator [+]
• Multiple (but invalid) exponents [4x^^^2]
• Multiple (valid) exponents [4x^2^3]

@dawnandrew100
I would need clarification as to how implied multiplications are to be handled here,
2/4x can be handled in two ways, 2/(4*x) or (2/4)*x. I would prefer if expressions deliberately required * sign. As it would resolve the ambiguity.

@shri-acha i think in the case of 2/4x, it should default to being interpreted as 2/(4x) unless parentheses are present for it to be interpreted as (2/4)x.

The implied multiplication between the integer/float and variable is pretty standard, so I don't think we should require that there be an explicit *

Oh right, sorry about the confusion, I do realize I might've thought too much on it before.
As for the implied operation handling, I expect changes to the parser as a pass to add implied multiplication before ast generation. I found a good reference on handling it here: https://github.com/davedelong/DDMathParser/wiki/Implicit-Multiplication

hey @dawnandrew100 , I did add all the tests and made it works for all the valid conditions. I've also created new error variants within PolynomialError, Let me know if we can add any more variants.

We have to work making invalid expressions throw suitable errors, I'd like to know how we could approach on adding checks for errors in expressions.

@shri-acha I'll do a proper look through in the morning, but I did notice that this was an error in the tests

    #[test]
    fn test_multiple_exponents(){
        let expr="4x^2^3";
        let tok_str = lexer(expr).unwrap();
        let result = parser(tok_str, 0.0);
        println!("{:?}",result);
        assert!(matches!(result,Err(_)));
    }

But 4x^2^3 is a valid expression because it would evaluate to 4x^8

Sure, I'll fix this test, it seems I've messed up while copy pasting.

As for better assert matching, I think assert_matches! is a better fit but it requires the nightly build. But for now, I'll leave assert matching with the way it is.

I just saw the suggested structure of Ast from #30, I'll be just refactoring my exisiting parser to support suggested structure.

@shri-acha You don't necessarily need to follow that to a tee. The structure that you have now looks fine

The bigger thing with the structs would be the naming conventions and the implemented traits. So if you could rename your struct from Ast to PolynomialAst and impl PartialEq<Vec<f64>> for PolynomialAst

The structs should also implement the PolynomialTraits, but I'll probably open up a new issue for that. Feel free to also tackle that, but it would involve implementing these functions located in this file

pub trait PolynomialTraits {
    fn parse(input: &str) -> Result<Self, PolynomialError>
    where
        Self: std::marker::Sized;
    fn eval_univariate<F>(&self, point: F) -> Result<f64, PolynomialError>
    where
        F: Into<f64> + std::clone::Clone + std::fmt::Debug;
    fn eval_multivariate<V, S, F>(&self, vars: &V) -> Result<f64, PolynomialError>
    where
        V: IntoIterator<Item = (S, F)> + std::fmt::Debug + Clone,
        S: AsRef<str>,
        F: Into<f64>;
    fn derivate_univariate(&self) -> Result<Self, PolynomialError>
    where
        Self: std::marker::Sized;
    fn derivate_multivariate<S>(&self, var: S) -> Self
    where
        S: AsRef<str>;
    fn indefinite_integral_univariate(&self) -> Result<Self, PolynomialError>
    where
        Self: std::marker::Sized;
}

You already have the parser, but the evaluation, derivation, and integration for both univariate and multivariate expressions might be a lot to include in just one PR

@dawnandrew100.
I do understand, but I believe using current implementation, will lead to complicated future evaluation logic, I've already implemented it with Expr and it works better and also handles with invalid conditions. I'll be pushing those changes and I think, we could merge the change.

@dawnandrew100 , I don't understand impl PartialEq<Vec<f64>> for PolynomialAst, Could we deal with this in issue #30 or a new issue, I suppose ? It'd be better if you explain the context of this there so that we can merge this PolynomialAst implementation.

@shri-acha actually I misspoke! We don't need the impl PartialEq<Vec<_>> for this struct

That's part of SimplePolynomial and PolynomialExtended just so that some of the tests could pass since some of the functions originally returned a Vec<64> or Vec respectively so the tests were built around that. But we don't need that for the PolynomialAst struct

PolynomialTraits should be implemented, but I can open a new issue for that so that we can get this PR merged and to close the issue that this PR is addressing

@shri-acha
There are a few unused imports and unresolved dependencies:

error[E0433]: failed to resolve: use of unresolved module or unlinked crate `once_cell`
 --> /home/runner/work/spindalis/spindalis/spindalis_core/src/polynomials/ast.rs:3:5
  |
3 | use once_cell::sync::Lazy;
  |     ^^^^^^^^^ use of unresolved module or unlinked crate `once_cell`
  |
  = help: if you wanted to use a crate named `once_cell`, use `cargo add once_cell` to add it to your `Cargo.toml`

warning: unused import: `std::cell::RefCell`
 --> /home/runner/work/spindalis/spindalis/spindalis_core/src/polynomials/ast.rs:4:5
  |
4 | use std::cell::RefCell;
  |     ^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default

warning: unused variable: `val`
   --> /home/runner/work/spindalis/spindalis/spindalis_core/src/polynomials/ast.rs:307:17
    |
307 |                 val => {}
    |                 ^^^ help: if this is intentional, prefix it with an underscore: `_val`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

For more information about this error, try `rustc --explain E0433`.
warning: `spindalis_core` (lib) generated 2 warnings
error: could not compile `spindalis_core` (lib) due to 1 previous error; 2 warnings emitted

@shri-acha Checks are passing, so just let me know if you're ready to merge this PR and I'll do a proper review 👌

I'm ready you can review the code.