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

added arctan, arccos and arcsin functions and an example file using them #525

Merged
merged 6 commits into from
Mar 31, 2024

Conversation

rimjhimittal
Copy link
Contributor

Changes i have made:

  1. Using regular expressions in the following function to address the issue where the replacement code confuses "tan" with "arctan".

    def create_python_expression(expression_string: str = None) -> str:

  2. Added Functions for arctan, arcsin and arccos in standard.py.

  3. Added an example file called arc_functions.py that tests all the three functions.

  4. Addressed an import error for "types".

@pgleeson
Copy link
Member

Thanks @rimjhimittal. As opposed to adding a python script to demonstrate this in the examples folder, could you add to the test case here: https://github.com/ModECI/MDF/blob/main/tests/test_mdf_functions.py, with usage of your new functions to test that they behave as expected.

@rimjhimittal
Copy link
Contributor Author

rimjhimittal commented Mar 28, 2024

Hey @pgleeson , I have added the test cases. I ran them, they all passed. Should I delete the python script I added to the examples folder?

@pgleeson
Copy link
Member

Hi @rimjhimittal. Yes, please delete the python script and image from the examples folder and just use the test. A more comprehensive example showing all the standard built in functions can be added at a later date.

@rimjhimittal
Copy link
Contributor Author

Okayy, I have removed them both.

@pgleeson pgleeson merged commit 1c2ee26 into ModECI:development Mar 31, 2024
11 checks passed
@rimjhimittal rimjhimittal deleted the arc_func branch May 29, 2024 21:18
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

Successfully merging this pull request may close these issues.

2 participants