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

Expand elementary functions in math module #132

Merged
merged 8 commits into from
Nov 27, 2022

Conversation

ZibingZhang
Copy link
Contributor

@ZibingZhang ZibingZhang commented Nov 22, 2022

feature is completed, just needs more tests

Overview

Adds more functions which can be expanded.

Details

References

#65

Blocked by

None

@odashi odashi added this to the v0.3 milestone Nov 22, 2022
@ZibingZhang
Copy link
Contributor Author

Not sure how much test coverage is best, but I've decided it might be easiest to test the base expansion in function_expander_test (as well as in regression_test), and that testing that nested functions get expanded properly should be done in regression_test. This is so we don't have to deal with constructing asts by hand.

Not sure what the best way to test that for all these functions you need exactly n arguments. The last PR used the test method test_hypot_expanded_no_args, but it'd be nice if there was a way to specify any number of args except for n would raise an error.

Also maybe it would be nice to include an ast_utils.make_name, that way we can stop setting ctx=ast.Load() for every single one of them.

@ZibingZhang ZibingZhang marked this pull request as ready for review November 25, 2022 01:48
@ZibingZhang ZibingZhang requested a review from odashi as a code owner November 25, 2022 01:48
@ZibingZhang ZibingZhang requested a review from odashi November 27, 2022 10:58
Copy link
Collaborator

@odashi odashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@odashi odashi merged commit 4b37366 into google:main Nov 27, 2022
@odashi odashi added the feature label Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants