maths / moodle-qtype_stack

Stack question type for Moodle
GNU General Public License v3.0
138 stars 147 forks source link

stack_include_contrib not allowing error() #1129

Closed LukeLongworth closed 3 months ago

LukeLongworth commented 4 months ago

I've reached the point in the school year where I am back to vector calculus, and as such I was wanting to migrate some of my existing questions to use stack_include_contrib("vectorcalculus.mac") rather than defining the div and curl functions every time.

However, the div and curl functions in that file use error(), which obviously is forbidden in the question variables. I assume that the stack_include_contrib function simply runs the included code as if it were typed normally into the question variables.

I could easily just remove the error message, but I feel like it is important for users to see. Is it possible to have error messages from inclusions like this handled the same way we would normally handle errors?

aharjula commented 4 months ago

Please describe what you mean by "the same way we would normally handle errors", normally as in Maxima?

stack_include and stack_include_contrib do indeed just include the remote code into that place, and after that, it gets handled as if it was written there. The same applies to [[include]]. We really don't want to have this feature provide alternative security rules and thus things that you cannot do in normal "question-variables" you cannot do here. In some sense, this is due to the way the inclusion happens, i.e., the code gets placed exactly there and can thus access all the question context defined before (and in a sense after) the include (this, in particular, applies to included code that does not define functions or other capabilities but instead directly does common actions). Thus, it needs to be secure within the context, and the only way we can check that is by handling it as a part of that context. This is somewhat different from how one would deal with selectively loading a Maxima library, which is something we still do not support at the question level; if we ever do, though, it would require us to first ensure that everything that comes in from that library is "safe" in all contexts.

In any case, we really need to develop the process for adding things to the "contrib" directory and ensuring that those stay functional. That test case logic needs to be built up soon... we just cannot have stuff there that cannot actually be included from there.

sangwinc commented 4 months ago

Mea culpa @LukeLongworth, sorry that's just silly. I'll sort that out in the short term, and we'll figure out a way of permitting error trapping in user-contributed stuff....

sangwinc commented 4 months ago

@LukeLongworth, I also notice we don't have any s_test_case coverage for curl either! Adding some examples would be great if you're thinking about this topic. Also, further functions in this file would be most welcome.

Chris

LukeLongworth commented 4 months ago

@sangwinc I'll have a look next week!

I've been wracking my brains for more useful functions or routines but I've generally come up blank. Most of what we do is as simple as differentiating with clever set up. I'll have another stocktake of our questions when I look at the s_test_case stuff.

We're also planning to begin writing a full set of quizzes for a 2nd year linear algebra course around July, and I am planning to write a "linearalgebra.mac" file at some point unless someone gets to it first. I'll start a discussion thread here when I start in earnest.

LukeLongworth commented 3 months ago

@aharjula, what I meant was that I'd love for an error message to pop up in red text below the question variables like I would get if I misused any built-in functions, e.g. image I do understand that this simply doesn't make sense in the context of stack_include() simply pasting the code in as if the user has typed it themselves.

Now that the fix is applied, I see that the default error message is probably sufficient anyway! image

I think my solution will just be to write some documentation so that any confused users don't have to read source code to diagnose their problem. @sangwinc Would it make sense for me to create a new Inclusions folder within the Authoring section, move Inclusions.md into this folder, and then write a dedicated file for each .mac file? I think we would need to be careful to make it clear that inclusions like this always reference the current version on github, so local documentation should probably include a link to stack-assessment.org rather than relying on local updates.

aharjula commented 3 months ago

The former error would basically require type inference to work and as we really don't have that capability it is a bit hard to implement. The latter just comes from a runtime error and is thus easy.

As to documenting contrib-libraries. Doing it like that manually is naturally an option. However, there is a project ongoing where the whole STACK-Maxima library would be "compiled" from source to tests, somewhat modified code and documentation. Basically, at some point we will probably add comments to each function in some sensible format and run a script that produces a rather typical API documentation, that would probably be something we would prefer contrib-libraries to use as well, there are naturally means for people to add intro sections for the whole libraries. Basically, the same scripts would extract those s_test_cases and run them as a part of the unit-tests.