Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch between use of degrees or radians for trigonometric functions #129

Open
JacobClinton opened this issue Dec 20, 2022 · 4 comments
Open

Comments

@JacobClinton
Copy link

Could you add the ability to handle trigonometric functions with angles as degrees instead of radians?

Normal (radians):
cos(90) = -0.448...

Desired (degrees):
cos(90) = 0

It would be great if you could add this functionality as something you could enable using the ExpressiveOptions or something similar.

I have ways of working around it for now (using regex to replace the whole trig function with my desired value) but I'd really appreciate it if you could add this functionality within the parser.

@bijington
Copy link
Owner

@JacobClinton thanks for the suggestion, I suspect it could be achieved through the ExpressionOptions enum. In fact I would like to consider migrating this enum into a class and making it work similarly to how the more modern approaches when initialising a library so instead we would write something like:

var context = new Context(
    options =>
    {
        UseRadiansForTrigonometricFunctions = true;
    });

Something like the above but I will open this in a new issue.

On the topic of your request - in summary yes we could do it. I guess a quick further question I would ask is, do you wish to switch between it's uses in your application as it currently stands or just use a single approach? If it is the single approach a nicer workaround could be to replace the current functions with degrees support. To do this you can do something like:

var context = new Context();

context.RegisterFunction(
    "Cos",
    (p, v) =>
    {
        // p = parameters
        // v = variables
        return p[0].Evaluate(v); // Perform your degree based cos function here
    },
    true); // Important as it replaces the existing functionality

@JacobClinton
Copy link
Author

@bijington thanks for your response, my apologies for the delay in mine I had to put my focus on other projects. I don't believe I would ever need to switch between the two approaches but it is an interesting concept. I'll have a look at registering all 6 functions myself but an integrated approach for the future would be greatly appreciated.

I do have to admit, your current ExpressionOptions enum seems to have some clear limits and definitely isn't the most intuitive solution. If you could migrate this and add a UseRadiansForTrigonometricFunctions I'd be extremely grateful. I understand that this change may not be an easy one, but I would appreciate it if you could let me know if/when you have completed it and I'll start using it straight away.

Otherwise, I have been using your parser for the last month to create an internal company tool and it has been working like a charm. I appreciate everything you've done so far and I look forward to seeing how this develops in the future.

@bijington
Copy link
Owner

No need to apologise for a delayed response that happens to me all the time 😄 .

Yes sadly the ExpressiveOptions enum was an old approach that I wish I hadn't chosen. I am going to open an issue to migrate to a newer approach as I mentioned above so that we could potentially move forwards with a cleaner way of configuring the framework in the future.

@JacobClinton
Copy link
Author

That all sounds ideal. Hopefully the new methodology doesn't cause you too much trouble to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants