Fix some issues with the sage.pl macro. #522
Conversation
drgrice1
force-pushed
the
fix-sage-macro
branch
from
05af4a3 to
68245a5
Compare
First, there was an obvious typo that made the macro's ShowAnswerBlank option fail. Next, sage uses python3 which requires that the print function have its arguments parenthesized. This causes the record_answer method in the ShowAnswerBlank eq 'none' case to fail. Next, '<script src="$CellServer/static/jquery.min.js"></script>' was recently added to this macro before the loading of the sagecell javascript. The sagecell javascript internally loads its own jQuery instance, and so there is no need for that. Furthermore, that version of jQuery conflicts with the version currently in use by WeBWorK. Finally, the sagecell server does not correctly deal with jQuery. They recently added the usage of the noConflict method, but then they still use and modify the global $ variable. This conflicts with our jQuery usage. So our version of jQuery is cached and restored after the sagecell javascript is loaded and executed. See https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5011 for some of my discussion on this.
drgrice1
force-pushed
the
fix-sage-macro
branch
from
68245a5 to
6ec8b7d
Compare
There was a problem hiding this comment.
I used the test problem @drgrice1 posted in the forum thread https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5011#p15356
- Before the patch - warnings were issued. (Listed below.)
- After the patch - all OK - no warnings.
Warnings from before applying the patch:
PG question processing error messages
PG warning messages
------
No answer evaluator for the question labeled: sageAnswer
------
Answer evaluator sageAnswer was not executed due to errors.
========
taniwallach
requested review from
drdrew42,
Alex-Jordan and
pstaabp
and removed request for
pstaabp and
Alex-Jordan
|
Would appreciate someone else testing before we merge this. |
There was a problem hiding this comment.
I tested the same problem as @taniwallach with the same before and after results.
As an aside, is that problem correctly computing the answer? My random version created a 2x2 graph (four points in a square) and expect the answer for the graph radius to be 0. I would have guessed 2.
|
I have no clue as to the validity of the problem. I didn't look closely. I just took the problem that was posted on the forum, and made the sage part work. |
|
We have 2 approvals after testing the patch. I'm merging it now. |
First, there was an obvious typo that made the macro's ShowAnswerBlank option fail.
Next, sage uses python3 which requires that the print function have its arguments parenthesized. This causes the record_answer method in the ShowAnswerBlank eq 'none' case to fail.
Next, '<script src="$CellServer/static/jquery.min.js"></script>' was recently added to this macro before the loading of the sagecell
javascript. The sagecell javascript internally loads its own jQuery instance, and so there is no need for that. Furthermore, that version of jQuery conflicts with the version currently in use by WeBWorK.
Finally, the sagecell server does not correctly deal with jQuery. They recently added the usage of the noConflict method, but it was not done so correctly. This conflicts with our jQuery usage. So our version of jQuery is cached and restored after the sagecell javascript is loaded and executed. This is not the perfect solution, as it is theoretically possible for something else (another ready or window load function) to be executed during the time that the sagecell server code is loading. The only perfect solution is for the sagecell code to be fixed.
See https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5011 for some of my discussion on this.