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

Use parens again if negative is factored out. (#455) #546

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Mar 14, 2021

This PR address issue #455, which causes the display for some reduced formulas be incorrect (parens are missing). The expression evaluates properly, but doesn't print properly. This is due to a setting that was trying to prevent (-2)x from showing as -(2*x) when it is reduced.

The reduction works by pulling negatives out and cancelling when possible, so -5/(-2*x) becomes -(5/(2*x)), then -(5/(-(2*x))), then -(-(5/(2x))and finally5/(2x). But at the (-2x)to-(2x)reduction, the parens are suppressed (so it would print as-2x, and the further reduction didn't change that, so you got5/2xinstead of5/(2*x)`. This PR removes the no-paren marker when the negation is moved further out, allowing the parens to be included again.

@Alex-Jordan Alex-Jordan self-requested a review March 15, 2021 04:13
Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

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

I tested with the original problem and a few more expressions I thought might be affected. All I can see is that it fixes the original issue without any side effects. So I recommend for merging.

Copy link
Sponsor Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

This checks out for me too.

@Alex-Jordan Alex-Jordan merged commit e860e04 into openwebwork:develop Mar 15, 2021
@dpvc dpvc deleted the issue455 branch April 19, 2021 14:16
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.

3 participants