Late penalty plugin

[lgpage]

Framework for loading plugin functions.
Adds enhancement for #233

TODO list from the discussions below:

• CSS style class for late penalty in feedback template
• Create separate preprocessor for assigning penalties
• Utilize traitlets to import plugin customizations
• Add tests and fix issues
• Check / add docstrings
• Update docs

[lgpage]

@jhamrick this is what I've come up with so far (still a bit to add), let me know what you think.

[jhamrick]

This is really awesome! I'm super excited about this 🎉

I do wonder though if we want to necessarily define a custom plugin loader class, rather than relying on traitlets' built-in functionality. With traitlets, you can define class variables so that instances of the class are loaded, and the type of that instance is configurable. A good example is in nbconvert, where things like the writer and postprocessor are configurable instances: https://github.com/jupyter/nbconvert/blob/master/nbconvert/nbconvertapp.py#L181 e.g. for the writer:

    # Writer specific variables
    writer = Instance('nbconvert.writers.base.WriterBase',  
                      help="""Instance of the writer class used to write the 
                      results of the conversion.""", allow_none=True)
    writer_class = DottedObjectName('FilesWriter', config=True, 
                                    help="""Writer class used to write the 
                                    results of the conversion""")
    writer_aliases = {'fileswriter': 'nbconvert.writers.files.FilesWriter',
                      'debugwriter': 'nbconvert.writers.debug.DebugWriter',
                      'stdoutwriter': 'nbconvert.writers.stdout.StdoutWriter'}
    writer_factory = Type(allow_none=True)

    def _writer_class_changed(self, name, old, new):
        if new.lower() in self.writer_aliases:
            new = self.writer_aliases[new.lower()]
        self.writer_factory = import_item(new)

this will by default use the FilesWriter class for the writer but it can be customized by passing in any class name. So if you had a file in the current directory called my_custom_writer.py with a class called MyWriter, you could set writer_class = 'my_custom_writer.MyWriter' and then the writer variable will get automatically populated with an instance of the given class. So one option is that we could do something similar with late plugins. What do you think?

I have a few smaller comments that I'll leave inline, too.

[jhamrick]

This li tag should probably be given a class and then define the color using that class in the CSS at the top of the page.

[lgpage]

Agreed. Did this just to quickly test it.

[lgpage]

I do wonder though if we want to necessarily define a custom plugin loader class, rather than relying on traitlets' built-in functionality... So one option is that we could do something similar with late plugins. What do you think?

This makes sense. I'll definitely look into it.
This is the first time I've used traitlets and unfortunately the documentation is lacking. It has been slow going trying to figure things out, so thanks for pointing out this example.

[jhamrick]

No problem, and yeah, the traitlets stuff can be pretty confusing. I remember being incredibly confused when I was first learning about it maybe a year ago. There's at least a bit more documentation now than there was then! These links might also be helpful, and definitely let me know if you have any questions...

• http://traitlets.readthedocs.io/en/stable/using_traitlets.html#dynamic-default-values
• http://traitlets.readthedocs.io/en/stable/using_traitlets.html#callbacks-when-trait-attributes-change
• http://traitlets.readthedocs.io/en/stable/trait_types.html#classes-and-instances

Note that nbgrader still has some of the traitlets 4.0 syntax, while there was new syntax introduced in 4.1: http://traitlets.readthedocs.io/en/stable/migration.html which reminds me, I should open an issue to remind myself to migrate the traitlets stuff!

[jhamrick]

Aha, I also tracked down an example of using the Instance traitlet in nbgrader itself, if that's helpful too:

https://github.com/jupyter/nbgrader/blob/master/nbgrader/apps/formgradeapp.py#L78

This one is a little different (and simpler) from the writer example. I'm actually not totally sure why the writer example is so complicated, as it is possible to also pass just the string of a class name for authenticator_class which seems much simpler (see https://nbgrader.readthedocs.io/en/latest/configuration/jupyterhub_config.html#configuring-nbgrader-formgrade )

[lgpage]

@jhamrick I still need to add tests to check this, but it seemed rather easy to utilize traitlets as you suggested.

[jhamrick]

There is an enum trait type which might be good to use here to ensure user typos throw an error.

[lgpage]

Thanks :) I was looking for the best way to do this validation.

[jhamrick]

👍 This looks great! Let me know when you are ready for me to review the tests and docs.

[lgpage]

@jhamrick ready for review.

[jhamrick]

Just wanted to say really quickly that I think this looks great -- I've skimmed over it and will have a longer look at it on Monday (I am about to leave for a conference in Singapore, so I'll be offline for the rest of the weekend).

[jhamrick]

Another quick update -- so sorry that I haven't had time to finish the review for this. I am definitely going to try to get to it this week (hopefully tomorrow)!

[lgpage]

No worries. I have been swamped with work lately, so probably would not have been able to get round to any updates had you done it earlier :P

[jhamrick]

Since this is only used in one place, I would define it in the function where you use it rather than having it be a global variable.

[jhamrick]

This looks really great. I only have the one comment about that plugin variable -- if you address that then I'll merge! Looks like this will make it in to the 0.3.0 release, too 😄

[lgpage]

Awesome :D
I'll try update this PR this evening or over the weekend.

[lgpage]

Done