-
Notifications
You must be signed in to change notification settings - Fork 56
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
Add support for cmath #1511
Add support for cmath #1511
Conversation
/bot run coverage docs pylint pyccel_lint |
Hello again! Thank you for this new pull request 🤩. Here is your checklist. Please tick items off when you have completed them or determined that they are not necessary for this pull request:
|
/bot run docs pylint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be lines in this PR which aren't tested. Please take a look at my comments and add tests which cover the new code.
If this is modified code which cannot be easily tested in this PR please open an issue to request that this code be either removed or tested. Once you have done that please leave a message on the relevant conversation beginning with the line /bot accept
and referencing the issue.
Similarly if the new code cannot be tested for some reason, please leave a comment beginning with the line /bot accept
on the relevant conversation explaining why the code can't be tested.
super().__init__(z) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't tested. Please can you take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/bot accept
Code is only provided to ensure the docstrings are aware of the parameter names
super().__init__(r, phi) | ||
|
||
#============================================================================== | ||
# Dictionary to map cmath functions to classes above | ||
#============================================================================== | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't tested. Please can you take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/bot accept
Code is only provided to ensure the docstrings are aware of the parameter names
/bot run coverage |
Unfortunately your PR is not passing the tests so it is not quite ready for review yet. Let me know when it is fixed with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job ! Your PR is using all the code it added/changed.
Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think! |
Hey @pyccel/pyccel-dev ! @EmilyBourne has just created this great new pull request! Check it out and let me know what you think! |
/bot run docs |
/bot run pr_tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job ! Your PR is using all the code it added/changed.
Add support for the cmath library using similar methods to those used for the math library. Fixes #1422