AnalyticsProtocol check_start complications regarding starting tag

[albert12132]

See https://github.com/Cal-CS-61A-Staff/ok-client/blob/master/client/protocols/analytics.py#L66 for context.

Right now, the regex is designed to match "*** REPLACE THIS LINE ***" and anything after it; the intention is to be able to catch starter code that has a placeholder:

def strategy():
    "*** REPLACE THIS LINE ***"
    return 5

However, I didn't anticipate that this means if a student doesn't remove the "*** REPLACE THIS LINE ***", then OK will still think that they haven't started the question, even if they start writing code right beneath that line.

It's technically OK to require students to actually replace that line (i.e. remove it), but it probably shouldn't be required. Suggestions on how to fix this? Or is requiring students to remove the line an acceptable behavior?

[papajohn]

I thought the original plan was to render the starter code as

return 5 # replace this line

On Tue, Jul 21, 2015 at 3:53 PM, albert12132 notifications@github.com wrote:

See https://github.com/Cal-CS-61A-Staff/ok-client/blob/master/client/protocols/analytics.py#L66 for context.

Right now, the regex is designed to match "* REPLACE THIS LINE *" and anything after it; the intention is to be able to catch starter code that has a placeholder:

def strategy(): "* REPLACE THIS LINE *" return 5

However, I didn't anticipate that this means if a student doesn't remove the "* REPLACE THIS LINE *", then OK will still think that they haven't started the question, even if they start writing code right beneath that line.

It's technically OK to require students to actually replace that line (i.e. remove it), but it probably shouldn't be required. Suggestions on how to fix this? Or is requiring students to remove the line an acceptable behavior?

— Reply to this email directly or view it on GitHub https://github.com/Cal-CS-61A-Staff/ok-client/issues/113.

[qiaojianjack]

I think this shouldn't be a problem for the project templates. If a function has a piece of "place holder" code, then every line of that code should be followed by "# Replace this line" as follows:

def strategy():
    return 0 # Replace this line

The "*** REPLACE THIS LINE ***" is only used when there's nothing in the function template. See https://github.com/Cal-CS-61A-Staff/berkeley-cs61a/blob/master/src/proj/config.py for context.

However, when students write something after "*** REPLACE THIS LINE ***" without removing it, the current code would still detect the problem as "not started". I think we should change the regex to detect whether there is only the "*** REPLACE THIS LINE ***" marker in a question. What I did before was using re.fullmatch(), but that function only exists in Python 3.4 and after.

[qiaojianjack]

Also for the first point above: now we technically didn't enforce the usage of # Replace this line by code. That should probably be added to config.py.

[qiaojianjack]

A possible way to fix it would be changing RE_DEFAULT_CODE to match from the start of the string to the end:

RE_DEFAULT_CODE = re.compile(r"""
^\"\*\*\*\sREPLACE\sTHIS\sLINE\s\*\*\*\"$
""", re.X | re.I)

That detects if "*** REPLACE THIS LINE ***" is the only content in the function body.

[qiaojianjack]

@albert12132 Should I push a quick fix to it?

[albert12132]

Your suggestion sounds fine, but it wouldn't be able to detect cases where there is ALT code after the "*** REPLACE THIS LINE ***" line. That doesn't break anything for the student, so I'm okay if you do that; just realize that you might have gaps in your data.

[qiaojianjack]

Ah I think that is a bug in config.py. When there is ALT code for a function, we shouldn't use "*** REPLACE THIS LINE ***" at all, but only use # Replace this line after the ALT code. I think I will post a change later to both config.py and this one.