ENH+API: Add functional interface

[scottclowe]

Add a new functional interface, fissa.run_fissa(images, rois, ...).

The functional interface is easier to use than the class/object based interface, especially for those not so experienced at coding. But it doesn't allow the flexibility of working with the raw traces, ROIs, etc.

• New example notebooks and scripts for this interface are added.
• The concise example in the README is changed to use fissa.run_fissa instead of fissa.Experiment.
• Tests for the functional interface are also added in the PR.

This PR is not currently passing the CI test suite because it exposes a pre-existing bug. The bug is - fissa does not save all of its outputs to the cache (in particular, the mean image). So when you run fissa and load up a cached output instead of recomputing, this information is not available. Since the new Functional notebook uses the same output directory as the Basic (OO) notebook, and the smoke tests do not clear the state of this directory between testing notebooks, this causes the Basic (OO) notebook to break when it loads the already-processed data instead of recomputing it and then tries to plot the mean image.

I am working on another PR which changes the way fissa saves its cache so it includes more outputs. That will resolve this issue.

[swkeemink]

I recommend we add two sentences about functional VS object-oriented notebooks, and how the interfaces are different.

Perhaps we can make the functional one the basic notebook (Recommended for first users), and the object oriented one the notebook we recommend to have more control (+ 1/2 examples of the kind of control).

[codecov]

Codecov Report

Merging #169 (94f93f7) into master (0a2548b) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage   93.09%   93.20%   +0.11%     
==========================================
  Files           8        8              
  Lines         912      927      +15     
  Branches      189      194       +5     
==========================================
+ Hits          849      864      +15     
  Misses         32       32              
  Partials       31       31              

Flag	Coverage Δ
nbsmoke	66.01% <75.00%> (+2.09%)	⬆️
unittests	93.09% <100.00%> (+0.11%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
fissa/__meta__.py	100.00% <100.00%> (ø)
fissa/core.py	96.80% <100.00%> (+0.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a2548b...94f93f7. Read the comment docs.

[scottclowe]

The documentation for the stable version, v0.7.2, points to the notebooks being served by our master branch.

I think it could cause a lot of confusion if the Basic usage notebook which is linked to swaps to using the functional interface before that functionality is released on PyPI, so I've taken out the commit which makes the functional notebook the "Basic usage" notebook and moves the current Basic usage to an object-oriented name.

The notebooks should be swapped over just before the next release.

[scottclowe]

I am now ensuring the output directory is unique whenever running the main notebooks by generating a unique output directory using the current datetime.

This prevents the issue where the same output directories are used for both notebooks, and the data is reloaded instead of run from scratch. This PR is no longer dependent on #177 being merged first.

[swkeemink]

The new notebook is very easy to use. Unlike the other notebooks it uses matplotlib. Would you want me to replace it by holoviews use to make it more consistent with the other notebooks, and add in a similar mini-tool for exploring the example traces? Or conversely, to replace the other notebook's holoviews usage by matplotlib. Also happy to leave all of it as is.

Secondly, the basic_usage.html is missing from this branch, but that is because of #178 perhaps?

Some comments on the functional interface tutorial:
# Note: you *must* use a different folder for each experiment.
Should explain why you must do this (so data doesn't get replaces)

# If you change the experiment parameters, you need to change the cache directory too

Same here, should add 'to avoid replacing existing analyses' or some such.

[swkeemink]

Additional comment on the notebook

You can optionally provide FISSA with the name of a cache directory. If this is given, FISSA will load any output that is present from there instead of recomputing it.

change to

You can optionally provide FISSA with the name of a cache directory. If the folder does note exist or is empty, FISSA will save the main outputs in a cache file. If the cache already exists FISSA will load any output that is present from there instead of recomputing it. The cache can also be loaded separately without calling FISSA.

Together with my comment in #177 (review) there could then be a short description of how to interact with the cache without using FISSA directly.

[scottclowe]

The new notebook is very easy to use. Unlike the other notebooks it uses matplotlib. Would you want me to replace it by holoviews use to make it more consistent with the other notebooks, and add in a similar mini-tool for exploring the example traces? Or conversely, to replace the other notebook's holoviews usage by matplotlib. Also happy to leave all of it as is.

I think it is better to use matplotlib. I think using holoviews makes the notebooks less approachable, since people are much less likely to be familiar with it. The notebooks are there to help people learn how to use fissa, and it is not conducive to that for them to have to try to understand what is going on with holoviews. Even if they are not familiar with matplotlib, the UI for matplotlib is easier to understand that holoviews. For example, what does it mean to multiply and add plot objects together? It is not practical to interpret without already having a good understanding of holoviews. Meanwhile the matplotlib commands plt.xlabel() are more verbose and you can follow along even if you are not very familiar matplotlib. Also, we do think will have users coming to fissa who are new to python but familiar with matlab; since matplotlib mimics the format of matlab plotting, we expect some users to be familiar with the syntax even when they aren't too hot with python.

However, the output from holoviews is better than matplotlib because it makes an object that you can scroll through. So I am happy to include it, but I think it should be matplotlib first, holoviews second. We are considering the Functional notebook to be the new Basic notebook, and the Object-oriented notebook to be the advanced notebook. So I think it is fine for the Basic notebook to be matplotlib only, and the advanced notebook can show both matplotlib and holoviews interaction.

For the other notebooks (sima, suite2p: #181) I think it would be better to stick with just matplotlib. The important thing is to indicate how to interact with the other tools, without a learning curve on other things in the notebook.

Secondly, the basic_usage.html is missing from this branch, but that is because of #178 perhaps?

I'm not sure which file you are referring to, sorry!

basic_usage_func.py is added, as it should be.

I see that "examples/Basic usage - Functional.html" was added, but should not be! I will have to refactor and remove it (I thought I already had done that).

"examples/Basic usage.html" certainly should not be added, because I am not yet overwriting the Basic notebook with the functional interface. See my previous comment - #169 (comment).

Some comments on the functional interface tutorial:
# Note: you *must* use a different folder for each experiment.
Should explain why you must do this (so data doesn't get replaces)

# If you change the experiment parameters, you need to change the cache directory too

Same here, should add 'to avoid replacing existing analyses' or some such.

Sure, that would have been better. I made a small improvement on the previous text, but did not bother to do to much improvement to this bit. The text will need to change when the API improves soon to automatically determine whether or not to load a cache file based on whether the parameter contents of the cache match.

[swkeemink]

I was going through all the notebooks, and was imagining being a new user. The more I think about it, the less I like the functional interface + its notebook as a first use as it is currently.

The most likely use-case a for a new user is someone who wants to explore using FISSA, and easily inspect how well the results are working. For this it is essential to be able to compare the raw extracted traces to the decontaminated traces. I would therefore suggest to change the function call to also return the raw traces, as follows

raw, result = fissa.run_fissa(images_location, rois_location)

or as an optional output

raw, result = fissa.run_fissa(images_location, rois_location, return_raws=True)

This makes it less elegant but I find this to be more likely to be useful for a new user. It would also make the results that are plotted in the functional notebook more striking as you can immediately see the differences.

As it is they will go to the functional notebook, find it hard to assess how well FISSA is working, struggle with how to do that... and then eventually find a small note that one has to use the object oriented interface.

EDIT: will make a PR of this if you agree

[scottclowe]

Correction: as an optional output, that would be

result, raw = fissa.run_fissa(images_location, rois_location, return_raw=True)

because you should always add optional return values after existing returns. And the parameter would be return_raw not return_raws because the output is called raw.

I see your point that new users will want to inspect the raw traces along with decontaminated traces.

However:

1. I think the interaction with calculating deltaf is not good.
2. We need to consider what the goal of the functional interface is.

Under your proposal, what is the behaviour with return_deltaf=True?

Is it like this?

deltaf_result, deltaf_raw = fissa.run_fissa(images_location, rois_location, return_raw=True, return_deltaf=True)

In this case we need to rename the return_deltaf argument because it is changing the output instead of (like return_raw) appending outputs and one would expect these arguments to do the same thing.

Alternatively it could be like this:

result, raw, deltaf_result, deltaf_raw = fissa.run_fissa(
    images_location, rois_location, return_raw=True, return_deltaf=True
)

which is an unwieldy amount of arguments to return.

The goal with any function is to do take input, process it (doing one atomic thing), and return the result (without side-effects). I think it is already a bit awkward that we compute deltaf as part of fissa.run_fissa. If we are having fissa.run_fissa return raw, I would like to spin off a separate function to compute deltaf, which takes raw and result as its arguments which can be called after fissa.run_fissa. This makes much more sense from the perspective of functions. However, because the fissa.Experiment state would be lost between these two function calls, we would not be able save the deltaf values to cache. I think this is fine, and would be motivation to remove caching functionality from fissa.run_fissa. I don't think that loading from a cache is intuitive behaviour when calling a function so this would also make sense as a change.

The function interface, including computing deltaf, would then look like this:

result, raw = fissa.run_fissa(images_location, rois_location, return_raw=True)
deltaf_raw = fissa.compute_deltaf(raw, fs=10)
deltaf_result = fissa.compute_deltaf(result, raw, fs=10)

with a new function

def compute_deltaf(result, raw=None, fs=None, across_trials=True):
    ...

that will use the raw argument to determine the baseline, if it is provided.

Because the new fissa.compute_deltaf function would have the same behaviour on calls to both result and raw, the deltaf_raw will include deltaf traces for the neuropil, which I think is also a fine change to make.

Let me know what you think to this.

Edit: Here the fs argument to fissa.compute_deltaf is a required keyword argument that appears after the optional raw argument because that way you can do this one-liner more easily:

deltaf_result = fissa.compute_deltaf(*fissa.run_fissa(images_location, rois_location, return_raw=True), fs=10)

but if preferred we could have fs be an arg instead of a keyword arg with default None.

[swkeemink]

Good point about interaction with return_deltaf.

I don't think that loading from a cache is intuitive behaviour when calling a function so this would also make sense as a change.
Yes agreed.

Having fs as an argumetn is not a problem. People generally should not use the default value there anyway.

So overall happy with your new follow-up suggestion.

[scottclowe]

Good point about interaction with return_deltaf.

I don't think that loading from a cache is intuitive behaviour when calling a function so this would also make sense as a change.
Yes agreed.

Having fs as an argumetn is not a problem. People generally should not use the default value there anyway.

So overall happy with your new follow-up suggestion.

Great. Which of us will implement this?

[swkeemink]

I'm going to make a start of it now

You also posed another discussion point:

We need to consider what the goal of the functional interface is.

There's two ways to think about it for me (and they're not mutually exclusive) .

(A) it's a simpler interface to ease people into using FISSA, and to be able to get started faster

(B) It's a simpler interface that's useful when integrating FISSA into existing workflows.

I think for both it is very useful to be able to keep the raws, although less important for B.

[swkeemink]

I'm thinking some more on this.

The caching is not very function-esque or python-esque. However, for an experimentalist using this it is very useful and intuitive I think.

Also, I don't like having to do several function calls. If we now separate delta_f out of the function, and users want to export these new results to matlab, we would basically have to write another function for matlab exporting to be able to export delta_f.

I think at that point it starts making little sense to use the functional interface as it starts to be as complicated as the object oriented version.

I wonder if we shouldn't return the raw values by default, and then apply the delta f to both, so:

deltaf_result, deltaf_raw = fissa.run_fissa(images_location, rois_location, return_deltaf=True)

and

result, raw = fissa.run_fissa(images_location, rois_location, return_deltaf=False)

And leave everything else as it is now.

I don't think it's that un-intuitive to output both the results and the raws.

I've done the above in a draft PR: #237
I suggest we move further discussion to there!