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

Added doc about inclusion of packages mentioned in the student's subm… #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vkgurbani
Copy link

…ission in the test environment

@tbrown122387
Copy link
Owner

There's a small issue here. Technically "[t]herefore, any libraries included in the student submissions must also be included in the test environment" is not correct.

Strictly speaking, this would be unfortunate if it was true. In that case the instructor would have to match and use the exact same libraries that every student is using. In a big class, that would be especially difficult. So the modularity is a perk.

For instance, if students are using dplyr to create tibbles, I can check the shape and size and particular elements without needing to library(dplyr) in the test file.

However, extra library() calls become necessary when your test code say calls functions from a third party library. Checking values created by students does not require third party libraries, but recreating them in the test file sometimes does.

In the example code you emailed me, your test files ultimately required a few extra library() calls because your test code was calling functions that used third party libraries.

Also, it should be mentioned that instead of using library() you could also prefix certain function calls with the name of their library. This would be difficult in the case of the test cod eyou sent me, I think, but I think it should still be mentioned if we were to change the vignette.

@tbrown122387
Copy link
Owner

Happy to pulll after some modifications have been made

@vkgurbani
Copy link
Author

vkgurbani commented Nov 4, 2024 via email

@tbrown122387
Copy link
Owner

Option 2 is better, but regards to your example, the problem with it is that your test code is assuming things about the student implementation, which is not a great idea. The student can surprise you by either not defining or, or changing it to somethign expecting.

At the very least, you should re-define this function you're calling in your own test code. Then, when you redefine it, you'll see you're clearly using third party libraries, and you can deal with them either by using library() or by using ::.

Even better, check the output without using the function at all.

"Option 2 works for me, but I understand that the instructor would have to match and use the exact same libraries that every student is using. "

Not so. The instructor can test student output without using all of those libraries. The student is limited to using libraries that are already installed on whatever machine the code is being run on. This is a feature as well. It forces students to minimize dependencies.

@vkgurbani
Copy link
Author

Option 2 is better, but regards to your example, the problem with it is that your test code is assuming things about the student implementation, which is not a great idea. The student can surprise you by either not defining or, or changing it to somethign expecting.

The same problem --- i.e., the student surprising you --- exists even in the case when pre-determined names of global variables are used; I don't see much of a difference. When I give the homeworks, I am meticulous to state the expected return value, for example: "(v) [0.33 points] How many frequent itemsets are there with a support of 0.10? Your function should return an object of class itemset."

At the very least, you should re-define this function you're calling in your own test code. Then, when you redefine it, you'll see you're clearly using third party libraries, and you can deal with them either by using library() or by using ::.

Even better, check the output without using the function at all.

I am not sure what you mean by "re-define this function you're calling in your test code.". Perhaps a quick example using the code I provided may explain your thought better.

Thanks,

  • vijay

@tbrown122387
Copy link
Owner

tbrown122387 commented Nov 4, 2024

Yes, at the very least, you need to request specific variable names. If students, say, spell them wrong, that's on them.

Requesting types is more strict. R is dynamically typed, so usually a few types can be returned and still pass my checks (e.g. tibbls and data frames are both fine).

If you asked them to define a function called q_1_a, and you're testing it, then yes, you need to expect at the very least a certain signature (i.e. what arguments it takes and in what order). In this case you would not redefine the function in your test file.

If q_1_a is some demo code that you allow them to make use of, but cannot force them to use, then I would redefine it again in your test file, just to be sure that it exists in the way you expect when you use it to test.

Anything you use in your test file, if it is dependent on a third party library, I don't see how you can expect to not have to use library() or something like this. If you let student code dictate what libraries are read in, it would be an absolute nightmare. Actually, the first time I wrote an autograder, I did this, and it was absolutely terrible.

@vkgurbani
Copy link
Author

If you asked them to define a function called q_1_a, and you're testing it, then yes, you need to expect at the very least a certain signature (i.e. what arguments it takes and in what order). In this case you would not redefine the function in your test file.

If q_1_a is some demo code that you allow them to make use of, but cannot force them to use, then I would redefine it again in your test file, just to be sure that it exists in the way you expect when you use it to test.

Right, in my case, q_1_a() is NOT demo code; it is the name of a function that they are instructed to write as part of their homework. The homework writeup stipulates (1) the name of the function, (2) any parameters it takes, and (3) the return value.

Anything you use in your test file, if it is dependent on a third party library, I don't see how you can expect to not have to use library() or something like this. If you let student code dictate what libraries are read in, it would be an absolute nightmare. Actually, the first time I wrote an autograder, I did this, and it was absolutely terrible.

Agreed; the intent is not to have the student code dictate libraries to be used to reduce the complexity on the auto-grader. So, by explicitly importing the library in the auto-grader code, I can continue using it and allow the students to modularize their code.

Question is, how should we update the gradeR document to support such a use case, assuming we want to support it. The only use case documented in the vignette right now is the use of pre-determined global variables. I am trying to stay away from such global variables due to unintended consequences and side effects that may creep into all but the most simple of programs.

Thanks,

  • vijay

@vkgurbani
Copy link
Author

@tbrown122387, good morning. Any more thoughts on the above chain?

One way to proceed is to expand the gradeR vignette so it supports multiple ways to structure homeworks for auto-grading. Currently there is only one way to do so --- using pre-determined and agreed-upon names of variables available in the global namespace. This may not work for all cases, so we could present a second way to use the package as I have done.

WDYT?

Thanks,

  • vijay

@tbrown122387
Copy link
Owner

I’m thinking more of a minimal edit. Maybe an additional sentence or two

@vkgurbani
Copy link
Author

vkgurbani commented Nov 13, 2024 via email

@tbrown122387
Copy link
Owner

A whole vignette? Is your approach that distinct from mine?

@vkgurbani
Copy link
Author

A whole vignette? Is your approach that distinct from mine?

@tbrown122387, Good morning. Well, the three sentences I suggested couple of weeks ago in the pull are:

"Note that the student submissions are run in a new, clean environment. The tests are run in a different environment than the student submissions. Therefore, any libraries included in the student submissions must also be included in the test environment."

However, these did garner some pushback [1], which is why we are having the current conversation. If you think we can wordsmith the above sentences, that'll be one way to proceed. Any suggestions on wordsmithing the above?

Thanks,

  • vijay

[1] #22 (comment)

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