Matrices and disabling showTypeWarnings

[somiaj]

I am unable to disable showTypeWarnings for matrices. I have matrices that sometimes are formulas and sometimes are not, and I want to disable the type warnings with non formula matrices. Consider the following MWE.

DOCUMENT();
loadMacros(qw{PGstandard.pl PGML.pl});
Context('Matrix');
$M = Matrix([[1], [2], [3]])->cmp(showTypeWarnings => 0);
BEGIN_PGML
[_]*{$M}
END_PGML
ENDDOCUMENT();

If you enter in the matrix [[x], [x], [x]], you get warnings that the answer isn't a number. My understanding is showTypeWarnings => 0 should disable that message, but it did not.

[dpvc]

The issue stems from the fact that for matrix arrays, the entries are checked using cmp() calls on the individual entries, not via the main $M->cmp. These are at

pg/lib/Value/AnswerChecker.pm

Line 723 in 30aa595

$data->[$i][$j]->cmp(@ans_cmp_defaults, mathQuillOpts => $ans->{mathQuillOpts})->evaluate($entry);

and you can see that it does not have the options that were in the original cmp() call.

One possible work around would be

diff --git a/lib/Value/AnswerChecker.pm b/lib/Value/AnswerChecker.pm
index fb4dacb0..c35251ca 100644
--- a/lib/Value/AnswerChecker.pm
+++ b/lib/Value/AnswerChecker.pm
@@ -96,6 +96,7 @@ sub cmp {
                correct_ans              => $correct,
                correct_ans_latex_string => $correct_latex,
                correct_value            => $self,
+               original_cmp_options => {@_},
                $self->cmp_defaults(@_),
                %{ $self->{context}{cmpDefaults}{ $self->class } || {} },    # context-specified defaults
                @_,
@@ -720,7 +721,7 @@ sub ans_collect {
 # This really only needs to know if MathQuill is disabled or not, but it is more efficient to just pass on the reference.
 # The value is safely ignored if $ans->{mathQuillOpts} does not match /^\s*disabled\s*$/i.
                        my $result =
-                               $data->[$i][$j]->cmp(@ans_cmp_defaults, mathQuillOpts => $ans->{mathQuillOpts})->evaluate($entry);
+                               $data->[$i][$j]->cmp(@ans_cmp_defaults, mathQuillOpts => $ans->{mathQuillOpts}, %{$ans->{original_cmp_options}})->evaluate($entry);
                        $OK &= entryCheck($result, $blank);
                        $ans->{typeError} = 1 if $result->{typeError};
                        push(@row, $result->{student_formula});

which saves the original options and passes them on to the entry cmp() function. Not elegant, but it is not easy to separate out the appropriate options from $ans itself, since they are mixed in with all the other properties in $ans.

[somiaj]

I was thinking the issue was something like that where the option wasn't being passed to the individual matrix entries correctly. But I wouldn't have any idea how to best pass them, so thanks for the patch.

Looking here, it appears that mathQuillOpts also had a similar issue, and was explicitly added here, and I was wondering if maybe it would already be included in original_cmp_options and not need to be explicitly included with this suggested change anymore.

Grepping the code for mathQuillOpts show that this is mostly set by the problem, so that entry would already be part of the original_cmp_options. Though in a few cases mathQuillOpts is added via a $context->flag (parserLogb.pl and parserRoot.pl), and another few cases it is part of cmp_defaults (parserWordCompletion.pl and contextBoolean.pl).

So I was wondering if it might make sense to also pass the defaults through with any additional options and this would include any mathQuillOpts as well. What do you think of this patch?

diff --git a/lib/Value/AnswerChecker.pm b/lib/Value/AnswerChecker.pm
index 3ed4f185..5cdfa053 100644
--- a/lib/Value/AnswerChecker.pm
+++ b/lib/Value/AnswerChecker.pm
@@ -96,6 +96,7 @@ sub cmp {
                correct_ans              => $correct,
                correct_ans_latex_string => $correct_latex,
                correct_value            => $self,
+               original_cmp_options     => { $self->cmp_defaults, @_ },
                $self->cmp_defaults(@_),
                %{ $self->{context}{cmpDefaults}{ $self->class } || {} },    # context-specified defaults
                @_,
@@ -716,11 +717,9 @@ sub ans_collect {
                                $entry = $ans->{original_student_ans};
                                $ans->{student_formula} = $ans->{student_value} = undef unless $entry =~ m/\S/;
                        }
-# Pass the mathQuillOpts on to each entry to ensure that the correct parsing is used for each entry.
-# This really only needs to know if MathQuill is disabled or not, but it is more efficient to just pass on the reference.
-# The value is safely ignored if $ans->{mathQuillOpts} does not match /^\s*disabled\s*$/i.
+                       # Also pass any cmp_defaults and options passed to the original cmp method.
                        my $result =
-                               $data->[$i][$j]->cmp(@ans_cmp_defaults, mathQuillOpts => $ans->{mathQuillOpts})->evaluate($entry);
+                               $data->[$i][$j]->cmp(@ans_cmp_defaults, %{ $ans->{original_cmp_options} })->evaluate($entry);
                        $OK &= entryCheck($result, $blank);
                        $ans->{typeError} = 1 if $result->{typeError};
                        push(@row, $result->{student_formula});

Maybe @drgrice1 has thoughts on if there is a nicer way to include what options to pass on from the original cmp method.

[somiaj]

It looks like passing original_cmp_options with @dpvc original suggestion (or mine) isn't going to work and will break things. The biggest being custom checkers, checker => sub {}, being passed to the elements of the matrix answer breaks things in strange ways. There are probably others too.

We will probably have to do similar with mathQuillOpts and pick only the options we want to pass on.