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

Make sure negative numbers are handled as a negated number in Value items (#658) #659

Merged
merged 1 commit into from Mar 12, 2022

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Mar 1, 2022

This PR fixes the problem where substituting a negative constant into a formula might not get the needed parentheses. It does so by providing a proper tree structure (negation together with a number) so that the parentheses are inserted when needed.

You can test using

Context()->variables->add(y => 'Real');
TEXT(Formula('x^y')->substitute(x => -3));

which should produce (-3)^y rather than -3^y, while

TEXT(Formula('x+y')->substitue(x => -3));

should produce -3+y (not (-3)+y).

Similarly,

TEXT(Formula('x^y')->substitue(x => 3));

should produce 3^y, as expected.

Resolves issue #658.

@pstaabp
Copy link
Sponsor Member

pstaabp commented Mar 4, 2022

This looks good. I also checked:

TEXT(Formula('x+y')->substitute(x => -3));

which returns x+(-3). I assume that is intended and not x-3.

@dpvc
Copy link
Member Author

dpvc commented Mar 4, 2022

@pstaabp, yes, this should produce x+(-3) just as Formula('x + -3') would. To get x-3 you need to use reduce.

@pstaabp pstaabp merged commit 1fa4116 into openwebwork:develop Mar 12, 2022
@drgrice1
Copy link
Sponsor Member

This pull request seems to be causing a problem with the LimitedNumeric context. Opening a problem using this context causes the system to hang. A minimal working example is attached.
MWE.pg.txt

@drgrice1
Copy link
Sponsor Member

@dpvc: The problem seems to be that the class method for a Parser::Legacy::LimitedNumeric::UOP::minus object returns 'MINUS' instead of 'UOP'. As a result when the Parser::UOP new method is called, the isNeg method of lib/Parser/Item.pm returns false when it is called at line 33 of lib/Parser/UOP.pm. Thus the Parser::Item::Value new method is called, which calls the Parser::UOP new method (in the change introduce here) and starts it all over again, thus causing an infinite recursion.

drgrice1 added a commit to drgrice1/pg that referenced this pull request Mar 29, 2022
…egacy::LimitedNumeric::UOP::minus package.

See openwebwork#659 (comment)
in that pull request for an explanation and minimal working example.
@dpvc dpvc deleted the issue658 branch March 29, 2022 13:25
pstaabp added a commit that referenced this pull request Mar 31, 2022
Fix an infinite recursion caused by #659 for the Parser::Legacy::LimitedNumeric::UOP::minus package.
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.

None yet

4 participants