latexify_py icon indicating copy to clipboard operation
latexify_py copied to clipboard

Addition / Subtraction Simplification for the stop parameter in sum / prod

Open Casper-Guo opened this issue 3 years ago • 2 comments

Overview

This PR addresses issue #74.

Details

Implementation

First check the stop parameter of range_info is ast.BinOp and represents an addition or subtraction. If so, the minus one is applied to BinOp.right and a new ast.BinOp node is created. Then the existing visit function is leveraged to create the needed Latex.

Note the special cases + 1 - 1 and - (-1) - 1 for which only BinOp.left is kept.

Added test cases to regression_test.py. Please let me know if these are more appropriate elsewhere (e.g. function_codegen_test.py)

Caveats

  • n+1-1 can be reduced to n but 1+n-1 cannot. This is because range(1+n) is unlikely and unusual. Support for this can be easily added if desired
  • chained additions and subtractions cannot be fully reduced. n + 3+4-1 will become n+3+3 not n+6. This is in line with how other chained BinOps are currently handled and may be addressed in a future issue

Questions

The implementation is slightly verbose and perhaps should be written as a separate function. Might other future PRs need this similar functionality?

References

Issue #74

Casper-Guo avatar Nov 16 '22 05:11 Casper-Guo

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Nov 16 '22 05:11 google-cla[bot]

@Casper-Guo Could you resolve the conflicts introduced by recent changes on main?

odashi avatar Nov 17 '22 00:11 odashi

@Casper-Guo It is not easy to select only your stuff. Try git merge main to remove duplicated changes.

odashi avatar Nov 17 '22 00:11 odashi

@Casper-Guo It is not easy to select only your stuff. Try git merge main to remove duplicated changes.

I rebased on main before pushing the new changes which should conceal all the new commits... Not sure why it didn't turn out that way. I will try to figure out a way to tidy it up.

Edit: Should be all set for your review now. Sorry for the confusion

Casper-Guo avatar Nov 17 '22 00:11 Casper-Guo

Looks great, thanks!

odashi avatar Nov 17 '22 03:11 odashi