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

Fix checks for scalar multiplication to not have to stringify the val… #330

Merged
merged 1 commit into from
Oct 14, 2017

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Oct 14, 2017

Fix checks for scalar multiplication (for points, vectors, and matrices) to not have to stringify the value if it is already a Real or Complex number. This resolves bug 3991. The issue was that when Context()->texStrings is in effect, the string match that was used to decide if the coefficient is a number would match against the TeX version of the number. When the number is large enough for scientific notation (i.e., more than 6 digits), then the TeX version of scientific notation would not match as a number, and would give an error.

To test the patch, use

Context("Matrix")->texStrings;
$P = Point(1,2,3);
$V = Vector(1,2,3);
$M = Matrix([1,2],[3,4]);
$a = Real(1230000);
$b = Real(123000);

sub TEST {
  my $test = shift;
  my ($result, $error) = PG_restricted_eval($test);
  TEXT($error? "$test: $error" : "$test: OK", $BR);
}

TEST('$a*$P');
TEST('$P/$a');
TEST('$a*$V');
TEST('$V/$a');
TEST('$a*$M');
TEST('$M/$a');

TEXT($HR);

TEST('$b*$P');
TEST('$P/$b');
TEST('$b*$V');
TEST('$V/$b');
TEST('$b*$M');
TEST('$M/$b');

Without the patch, all the lines involving $a should produce an error, but the ones with $b should be marked OK. With the patch, all the lines should be OK.

…ue if it is already a Real or Complex number.
@mgage
Copy link
Member

mgage commented Oct 14, 2017

Works fine. Thanks very much Davide. That removes an annoyance I've been working around.

@mgage mgage merged commit 9e0930c into openwebwork:develop Oct 14, 2017
@dpvc dpvc deleted the fix-number-check branch January 5, 2018 21:32
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.

2 participants