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

Properly handle negative constants and variables. #580

Merged
merged 1 commit into from
Jun 10, 2021

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Jun 6, 2021

This PR resolves the issue raised in the WeBWorK forum where the LimitedPolynomial context didn't rule out certain expressions that should not be allowed. These turned out to be cause by not handling negation properly.

To test, use

loadMacros("contextLimitedPolynomial.pl");
Context("LimitedPolynimal-Strict");
Compute("-2 + x + 4");
Compute("-x + x");

Both of the Compute() statements should produce error messages (without the patch, they will produce formula objects.

@Alex-Jordan
Copy link
Contributor

@drgrice1 Should this be retargeted to 2.16?

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.

Tested with Davide's suggestion, and I see the bug before this commit. After this commit, the bug is gone.

@drgrice1
Copy link
Member

I am getting Unknown context 'LimitedPolynimal-Strict' with your test code!

Other than that everything works as claimed.

I think this should be targeted for 2.16.

@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

Sorry, should be LimitedPolynomial-strict. (Lower case "strict")

@drgrice1
Copy link
Member

@dpvc: Yeah, I know. I guess the exclamation mark wasn't enough to indicate that I was kidding.

Thanks for the fix.

I am going to re-target this for PG 2.16 and merge it if that is okay. It will also be added to develop of course.

@drgrice1 drgrice1 changed the base branch from develop to PG-2.16 June 10, 2021 11:13
@drgrice1 drgrice1 changed the base branch from PG-2.16 to develop June 10, 2021 11:14
@dpvc
Copy link
Member Author

dpvc commented Jun 10, 2021

OK, sounds good.

@drgrice1
Copy link
Member

... or nevermind on the re-target. I will merge it, and then add it to PG 2.16. It seems this branch doesn't want to re-target cleanly.

@drgrice1 drgrice1 merged commit e239bdb into openwebwork:develop Jun 10, 2021
drgrice1 added a commit that referenced this pull request Jun 10, 2021
Properly handle negative constants and variables.
@dpvc dpvc deleted the fix-limited-polynomials branch January 3, 2023 07:43
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