aytekinar commented 6 years ago

Expected behavior

I would expect that the system to keep hiding the hidden tests in the feedback students receive.

Actual behavior

The system reveals the hidden tests in the generated HTML file. Is this a bug, or is this actually on purpose?

Steps to reproduce the behavior

Feedback on a student's assignment wich has hidden tests.

lgpage commented 6 years ago

@aytekinar this is by design. For what use case would you want to keep the tests hidden in the feedback?

jhamrick commented 6 years ago

Yeah, like @lgpage said that's intentional---the reasoning is that if you are grading students against hidden tests, it's still useful for the students to be able to see what they got wrong and understand why they got it wrong.

aytekinar commented 6 years ago

Thank you, both @lgpage and @jhamrick, for your responses. At least an option would have been great in that regard. When we allow for resubmissions, it might be better to hide the tests. Then, once the final grade is given, then the student can see what went wrong in the whole process. That would help when studying for the exams while keeping the hidden tests hidden in the intermediate submissions. I don't know, maybe our use case is not the best one :)

jhamrick commented 6 years ago

Ah I see, allowing for resubmissions is not something I'd considered.

I think it's probably reasonable to add an option to suppress hidden tests in the feedback, but have it be off by default, and with a warning that if the hidden tests change the state of the notebook it might be very difficult to understand the behavior of visible parts of the notebook.

aytekinar commented 6 years ago

Thank you for taking it as a feature/enhancement request. I cannot say anything regarding how you would implement and which default behaviour you would opt-in. But provided that there is a feature and that it is documented, I would (personally) be glad to read through and modify the settings accordingly.

Cheers :)

bbhopesh commented 5 years ago

HI @jhamrick. I need this feature too. Has anyone started working on this? I can start or jump in to help.

I also wanted to hear your thoughts on how to approach this technically. Cell metadata doesn't contain any information indicating if test case is hidden or not, so we probably can't do it in the feedback template. We can either add metadata and then take care of it in the template or do some preprocessing to create a copy of notebook with hidden cases removed. Which of the two would be better? or if you better ways in mind? If it is latter, what would be best place to do that preprocessing? feedback app or converter?

jhamrick commented 5 years ago

Off the top of my head, I think the way to do this would be to run the ClearHiddenTests preprocessor during the feedback step.

jhamrick commented 5 years ago

1156 brought up the of whether to keep the cell output or not. I think it might be hard to determine which output is a result of a hidden test and which is not, so I think maybe the easiest way to implement this anyways is to strip out the hidden tests from the code cell but retain all cell output.

debbieyuster commented 2 years ago

Another important use case for this feature: Instructor may want to reuse some exercises in a future semester, so may not want to hand out printed solution keys that can easily be passed down.

ykazakov commented 2 years ago

I think the hidden tests can be easily removed using a preprocessor as documented here. There is already a preprocessor for that, which is used for producing the student version of the notebook. Simply add to nbgrader_config.py:

c.Exporter.preprocessors = ['nbgrader.preprocessors.ClearHiddenTests']

The only problem is that, as pointed out in #1156 the content of the hidden tests is also exposed in tracebacks for error messages (shown when the tests fail). A workaround would be simply to remove all trackbacks from the feedback, which can be easily done by a custom preprocessor:

from nbgrader.preprocessors import NbGraderPreprocessor

class ClearTracebacks(NbGraderPreprocessor):
    def preprocess_cell(self, cell, resources, cell_index):        
        if cell.cell_type == 'code':
            for output in cell.outputs:
                if 'traceback' in output:
                    output['traceback'] = []
        return cell, resources

c.Exporter.preprocessors = ['nbgrader.preprocessors.ClearHiddenTests', ClearTracebacks]

ykazakov commented 2 years ago

Unfortunately c.Exporter.preprocessors applies to all notebook converters, e.g., for auto-grading. It is probably not a good idea to remove hidden tests from the auto-graded version. :-/

Fortunately, it looks like it it is possible to set preprocessors only for feedback using a more specific key c.GenerateFeedback.preprocessors. However, the default preprocessors GetGrades and CSSHTMLHeaderPreprocessor seem to be overwritten with this option, so one need to include them as well:

c.GenerateFeedback.preprocessors = [
    'nbgrader.preprocessors.GetGrades',
    'nbconvert.preprocessors.CSSHTMLHeaderPreprocessor',
    'nbgrader.preprocessors.ClearHiddenTests',
    ClearTracebacks,
]

ykazakov commented 2 years ago

A perhaps even better solution, would be to execute the notebook again after removing the hidden tests. This way, the output should be exactly the same as for the student version of the notebook:

c.GenerateFeedback.preprocessors = [
    'nbgrader.preprocessors.GetGrades',
    'nbconvert.preprocessors.CSSHTMLHeaderPreprocessor',
    'nbgrader.preprocessors.ClearHiddenTests',
    'nbgrader.preprocessors.Execute',
]

The downside of this approach is that generation of the feedback could take a bit longer, just like for auto-grading.

debbieyuster commented 2 years ago

@ykazakov Thanks for this helpful workaround. I tried putting your most recent code into a nbgrader_config.py file but got the following error:

[GenerateFeedbackApp | WARNING] Config option preprocessors not recognized by GenerateFeedback.

My entire nbgrader_config.py is as follows:

c = get_config()

c.GenerateFeedback.preprocessors = [
    'nbgrader.preprocessors.GetGrades',
    'nbconvert.preprocessors.CSSHTMLHeaderPreprocessor',
    'nbgrader.preprocessors.ClearHiddenTests',
    'nbgrader.preprocessors.Execute',
]

I do have another nbgrader_config.py file in another directory, but I don't think it's being accessed. Any chance you could quickly diagnose the problem? Presumably something important missing from my file?

ykazakov commented 2 years ago

@debbieyuster Oh, you are right, looks like the commit #1376 was not (yet) backported to branch 0.6.x. I am using nbgrader from the master branch, which seems rather stable.

debbieyuster commented 2 years ago

@ykazakov Thanks, I really appreciate your help! I tried installing from GH but there was an error during the build and my system seems to still be using the 0.6.x installation. I'm afraid to mess with things, especially mid-semester, so at this point I am going to give up on the post-processing...hopefully a new version of nbgrader will be released soon that includes the commit you referenced.

ykazakov commented 2 years ago

@debbieyuster Well, there are not many changes in the commit #1376. You can do them yourself:

locate the file generate_feedback.py in your python installation. For example, I have it under /opt/conda/lib/python3.9/site-packages/nbgrader/converters/generate_feedback.py

append .tag(config=True) at the end of line 25 so that it looks like that:

preprocessors = List([
    GetGrades,
    CSSHTMLHeaderPreprocessor
]).tag(config=True)

debbieyuster commented 2 years ago

@ykazakov Thanks, this is a great idea! It worked perfectly for the first student's notebook. This student's code had passed all my autograder tests. The subsequent notebooks all generated errors/infinite loops. I see that commit #1376 contains changes to multiple files so perhaps I'd need to make all (or more of) those changes, and/or perhaps the new and improved generate_feedback will only work on new assignments, not those I created and graded before making the changes.

If anyone else is trying this on Windows 10 and using Anaconda, my generate_feedback.py file was located here: C:\ProgramData\Anaconda3\pkgs\nbgrader-0.6.1-py38haa244fe_1\Lib\site-packages\nbgrader\converters\generate_feedback.py

Here are some of the errors I got when generating feedback through the Formgrader:

[WARNING] Config option `display_data_priority` not recognized by `ClearHiddenTests`.
[WARNING] Config option `display_data_priority` not recognized by `Execute`.
[...]
[ERROR] While processing assignment C:\Users\dyuster\autograded\1278\hw5, the kernel became unresponsive and we could not interrupt it. This probably means that the students' code has an infinite loop that consumes a lot of memory or something similar. nbgrader doesn't know how to deal with this problem, so you will have to manually edit the students' code (for example, to just throw an error rather than enter an infinite loop).

And tracebacks:

Traceback (most recent call last):
  File "C:\ProgramData\Anaconda3\lib\site-packages\nbgrader\utils.py", line 514, in capture_log
    app.start()
  File "C:\ProgramData\Anaconda3\lib\site-packages\nbgrader\converters\base.py", line 74, in start
    self.convert_notebooks()
  File "C:\ProgramData\Anaconda3\lib\site-packages\nbgrader\converters\base.py", line 413, in convert_notebooks
    raise NbGraderException(msg)
nbgrader.converters.base.NbGraderException: Please see the the above traceback for details on the specific errors on the above failures.

I'm not expecting anyone to fix this for me! Just sharing in case anyone is interested or in case it helps someone else.

For now I'll revert my code back.

debbieyuster commented 2 years ago

Wanted to revisit this - keeping hidden test contents out of student feedback should be doable now (see #1376 and #1519). What is the simplest way to configure this? Is it @ykazakov 's code above from 2021/10/27 or does #1519 lead to a different recommended way? Thanks in advance.

jhamrick commented 2 years ago

Just from a quick look I think it would still be the same as @ykazakov described.

On Sun, Aug 21, 2022, 2:21 AM Debbie Yuster @.***> wrote:

Wanted to revisit this - keeping hidden test contents out of student feedback should be doable now (see #1376 https://github.com/jupyter/nbgrader/pull/1376 and #1519 https://github.com/jupyter/nbgrader/pull/1519). What is the simplest way to configure this? Is it @ykazakov https://github.com/ykazakov 's code above from 2021/10/27 or does #1519 https://github.com/jupyter/nbgrader/pull/1519 lead to a different recommended way? Thanks in advance.

— Reply to this email directly, view it on GitHub https://github.com/jupyter/nbgrader/issues/917#issuecomment-1221441759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL5D2SVE7GYT2QQGEHF3V2GABLANCNFSM4EII7PYA . You are receiving this because you were mentioned.Message ID: @.***>

debbieyuster commented 2 years ago

It worked! Thanks to both of you!

AIRCAP commented 10 months ago

I used a slightly different config. As mentioned, the output of grading cells often leaks information about the code inside, especially when errors are thrown, which is the case if assertions fail. Re-executing the notebook alone didn't fix this for me, as this somehow did not change this. I had to add an additional line to first clear all outputs, and then re-execute the whole notebook. The code working for me was

c.GenerateFeedback.preprocessors = [
     'nbgrader.preprocessors.GetGrades',
     'nbconvert.preprocessors.CSSHTMLHeaderPreprocessor',
     'nbgrader.preprocessors.ClearOutput',
     'nbgrader.preprocessors.ClearHiddenTests',
     'nbgrader.preprocessors.Execute',
]

jupyter / nbgrader

Add option to keep hidden tests hidden during feedback step #917

Expected behavior

Actual behavior

Steps to reproduce the behavior

1156 brought up the of whether to keep the cell output or not. I think it might be hard to determine which output is a result of a hidden test and which is not, so I think maybe the easiest way to implement this anyways is to strip out the hidden tests from the code cell but retain all cell output.