check50.import_checks should rename checks

[Jelleas]

Something like:

check50.import_checks("../less", prefix="less_")

Right now it's too easy to break checks through naming conflicts.

This feature would require renaming in check50.runner._check_names.

There's also currently a weird dependency on the original function name, rather than the decorated one, within the dependency graph. This is caused because the CheckResults are created from the original function, and the dependency graph operates on these results. Down the line, this causes things to break if you rename the decorated function, but not the original one. We should change this such that everything uses the decorated function name as you would expect.

Here's a working example implemented in the checks:

https://github.com/minprog/cs50x/blob/2019/sentimental/__init__.py

[cmlsharp]

Yeah, import_checks is a bit of a misnomer. Really it just gets you a module from a file path, after which you have to do from x import * or it doesn't work anyway.

What does this do for checks in the current module which depend on checks in the imported module? e.g. resize/more has something like

import check50
less = check50.import_checks("../less")
from less import *
# exists defined in less
@check50.check(exists)
def ...

I agree we need some kind of namespacing tho.

[ivanharvard]

Perhaps it would be better just to import the module directly and then use Python's own . operator to access functions within that module. For instance,

less = check50.import_checks("../less")
import less

@check50.check()
def foo():         # this function is never accessed
    pass

@check50.check(less.foo)
def bar():
    pass

Now, the namespaces are clearly distinct. It arguably looks "less clean" than doing from less import *, but I think the check writer can make the final decision on what specific functions they are okay with including in the namespace (i.e. from less import foo, bar, etc).

In either case, whether we decide to add a prefix argument to import_checks or just make them import the module directly, we are burdening the check writer with the responsibility of having "less clean" code (@check50.check(less_foo) is just as unclean as @check50.check(less.foo)) and the responsibility of maintaining an (un)cluttered namespace. So using Python's built in syntax would probably be better than having to rewrite each function name in both the import stage and the dependency graph stage.