Fix #62: add verbose option

[mwermelinger]

The new option reports:

• full pathname of config file
• units checked for each file
• total number of files checked and issues found
• total number of files skipped due to errors

Let me know if any other thing should be reported, but I think the above covers most things needed for debugging and knowing that a file was processed but has no errors.

The info is sent to stdout and I don't think algoesup will catch it because none of it is of the form filename:line: .... If students want reassurance that the cell was checked, then probably need a more generic approach in algoesup that prints allowed|ruff|pytype found no issues if no error message is matched.

[densnow]

Is it worth mentioning the number of cells that were skipped due to syntax errors when a notebook is checked?

Even though there is already a summary, I feel the addition of a further summarising sentence could be beneficial. E.g "All the code checked conforms to the expected Python subset. No further action required." or " Constructs have been detected from outside the specified Python subset. Review the list above for details and locations". I think for students not used to reading outputs from the CLI this might help to clarify things.

The verbose output drew my attention to the behavior of allowed when checking a TMA with the default filename e.g. allowed -v 23J_TMA01.ipynb. This will result in units 1-23 being checked. Of course this behavior comes from the get_unit() function and the 23 in the name (I think), but in this instance the name is causing unintended consequences. Could this be considered a liability when we have no control over the naming of such files?

[mwermelinger]

Thanks, but I think it would unnecessarily complicate the code and potentially confuse the students more:

• one would need to distinguish syntax errors due to no IPython from other syntax errors
• saying that no further action is necessary followed by e.g. a warning that -m was not used can be confusing

[mwermelinger]

The TMA instructions in the book explicitly mention using -u 10|20|30 for each TMA.

[densnow]

The TMA instructions in the book explicitly mention using -u 10|20|30 for each TMA.

Yes, but what if someone does not follow these instructions and did not use the -v option? The name of the file drastically alters the behavior of the script and one could read the whole "checking code" section (from the github docs) without knowing it. I am not sure what the benefit is of extracting the unit from the name?

Thanks, but I think it would unnecessarily complicate the code and potentially confuse the students more:

• one would need to distinguish syntax errors due to no IPython from other syntax errors

That's fair enough.

• saying that no further action is necessary followed by e.g. a warning that -m was not used can be confusing

You could leave out the part that says no further action is required. I guess my point was that some students have found the results hard to interpret and so when they now get

INFO: found 0 unknown constructs in 0 .py and 1 .ipynb files

IMHO it is still not that user friendly. They might not know what a .ipynb file is. Some may not know what a construct is (I can remember a post asking that question) I can imagine someone thinking "because it found 0 unknown constructs does that mean my TMA is OK?"

[mwermelinger]

If someone doesn't follow instructions, it's their problem, not mine. I need to extract the unit name to check the 100+ notebooks that comprise the M269 book in one go.

Fair point about the message. I've changed and pushed to the branch.

[densnow]

If someone doesn't follow instructions, it's their problem, not mine.

Yes I guess that's true :smile:.

I need to extract the unit name to check the 100+ notebooks that comprise the M269 book in one go.

That makes more sense now.