Make default plot directory customizable

[tbekolay]

Currently, if you do not pass a string to --plots, the default directory that plots will be placed in is called plots. In Nengo core we had some automated ways of setting this directory (using the Simulator fixture), but since this needs to work for any repository, we can't expect a Simulator fixture to exist.

A few ways we could think about automating the name of this directory:

1. Add a configuration option that you can set through the setup.cfg / pytest.ini file that sets the default plot directory name for a given project.
2. Somehow use the passed in args / configuration to generate a plot directory (so that if you pass in, e.g., --target loihi you get a different default plot directory name than if you pass in --target sim).

[hunse]

I would propose a combination of 1 and 2.

We have a configuration option for the directory. This can just be a simple string (this is option 1 above). However, we could also let this be some sort of template string, where you could for example put {target} and we'd call config.getoption('target') and put this value into the string at that location (this would allow for option 2).

Overall, I think this balances simplicity and customizability fairly well. Do people agree with this general approach? Of course, we can have discussions about the actual syntax for specifying template values in the string (perhaps we want something more jinja-like with double curly braces {{target}}).

The other option (that could either be used with the above, or as an alternative), is to allow users to provide a python function that returns the directory name. This would allow it to be even more customizable (for example, having directories based on fixture, which we used in nengo to group things both by simulator and by neuron type). This option didn't seem to have too much traction in our brief discussion this morning, though.

An alternative would be to extend the templating approach to allow arbitrary python code in the template, so you could do something like ``plots.{{request.getfixturevalue('nl').name}}` to put the nonlinearity in the directory name, for example. However, if we really want this kind of power, I think we'd be better served by just allowing arbitrary python functions; it'd be easier to implement and use, and more powerful.

[tbekolay]

I like the idea of having some way to template it, though I think it should be pretty restricted as I think this will be one of those things that 0.01% of users will use. So, the Python function for example seems like a difficult thing to implement correctly and maintain, and isn't worth it gives how few users I suspect will want it.

Some restricted templating in pytest config options sort of has precedent with log format messages; unfortunately, that's caused a few issues and resulted in pytest kind of giving up on using setup.cfg (see 1, 2, 3). It seems like pyproject.toml might be a fix for all for all of this, but in any case it might be a good idea to avoid % in our template despite it having some precedent. Using the {key} style makes sense to me, and has precedent with the Python str.format function.

[hunse]

Thinking about this a bit more, it's not clear how to do templating in a way that would actually be useful. My inspiration was what we do in nengo: https://github.com/nengo/nengo/blob/36070176f05da29ee896f27917a470d83e7d87c5/nengo/conftest.py#L101

To achieve something like that, though, we'd need a lot of control over how to format each template value. In that example, we use each fixture's __name__ since they're classes. We can't assume that will be the best, though; many fixtures might be instances, and str or repr would work better.

Personally, I think a Python function is a lot easier to support than a complex templating system, since for that we don't have to build anything (users just have to be familiar with pytest's API to get what they need and return the appropriate string). So I would vote either to either allow Python functions only (instead of the templating system), or to only allow strings in the .ini file (in which case, you'd be setting a fixed default that would be used if no other directory name is specified by the --plots argument).

[tbekolay]

Of those two options, I would vote for only allowing strings, as I think that'll cover 99% of the use cases anyway. Partly I lean that way because I'm not sure how the Python function approach would work. Are you exposing a new pytest hook that people can override? If not, how do people know what function to write, what it takes in and what it returns?

[hunse]

There's two ways I would do it.

One is just basically monkeypatching. So in plugin.py we'd have something like

def dirname(request):
    return "ourdefaultdirname"

def set_dirname_function(fn):
    Plotter.dirname_function = fn

class Plotter:
    dirname_function = dirname

then in their conftest.py they'd call set_dirname_function with their own function.

The other option would be to have a plt fixture factory. In plugin.py:

def make_plt_fixture(dirname_fn):
    @pytest.fixture
    def plt(request):
        # do some stuff to return a Plotter using the dirname_fn

then in their conftest.py they'd call make_plt_fixture with their dirname function. We'd have to specify in the docs how to define a dirname function (arguments, returns, etc.), though I assume we'd have a default one that could be an example.

[tbekolay]

The monkeypatching could work, it relies a lot on documentation but since very few will want to do it anyway seems like no harm to make it possible to patch if people want to (we can even expose a function that does the monkeypatch so people don't need to know about the Plotter class).

I don't think the plt fixture factory will work (though I'm not 100% sure) because when you pip install pytest-plt, I assume we will ship a plt fixture for people who don't want to override the directory name. So if people then call the make_plt_fixture function you would have two fixtures with the name plt (unless it's renamed, but I think that would be too confusing and people would mistakenly still use plt).

[tbekolay]

Done in #13; the monkeypatching approach was also attempted in the directory-formatter branch if we decide to revisit that idea.