scidash / neuronunit

A package for data-driven validation of neuron and ion channel models using SciUnit
http://neuronunit.scidash.org
38 stars 24 forks source link

Move unit testing datasets elsewhere #216

Open rgerkin opened 5 years ago

rgerkin commented 5 years ago

@JustasB The morphology unit tests are currently failing because the .swc file they rely on is not being found. I think it is because it is not installed by pip, but just remains in the tarball. It is technically possible to configure the MANIFEST.in file to install it somewhere, but another option is just to have it stored elsewhere in some permanent form and have the unit test class's setup method just download it. I think I prefer the latter because data files don't really belong in neuronunit itself.

I'm not sure why the unit test was working before and is failing now, but in any case I think the long term solution is to have such supporting data files be outside of neuronunit.

JustasB commented 5 years ago

Seems like two separate issues:

1) Build fix The problem with the build had to do with not reverting to original working dir after from neuron import h failed on travis. i.e. this line did not get executed when neuron is not present, resulting in a cwd that was different from the expected repo root. I checked in the fix here: https://github.com/scidash/neuronunit/pull/217

2) Policy on where to keep unit tests and their files

The swc file is a declarative version of a unit test setup script. It would be possible to hard code something within the .py file (as often happens with very small setup scripts) and have the unit tests use that instead, but I chose the morpho unit tests to read it from a separate, later-updatable file.

IMO, if we have decided to keep unit test .py files in the repo, then we should keep files those unit tests depend on in the repo too.

I agree that items that end users don't care about like unit tests and their files should not be included in the final packaged pip wheels, but that can be done by adding exclusion filters to setup.py.

JustasB commented 5 years ago

This may need a larger discussion in general. As NU grows and more tests are being included, how much of new tests should be included within "core NeuronUnit" and which ones should be separate packages that people install on top of core-NU. I don't have an answer, but part-of-core vs. separate options come with their own sets of problems.

rgerkin commented 5 years ago

@JustasB Good catch on the fix. Your solution to not having neuron import (and load mechanisms) is much better than mine.

Also, I guess I'm fine keeping the .swc file with the unit tests. But there has to be some file size beyond which this policy is unreasonable. Say anything > 10 MB should be stored externally, or anything > 1 MB if it is a non-diffable binary file expected to change. The .swc file is only about 160KB.

For OpenWorm testing I set up optional installations like this following the guidelines for "extras" here. All of optimization, for example, could be considered an extra, even though it is important, simply because not everyone wants to do model optimization. So we could so something similar here.

JustasB commented 5 years ago

I agree that we should keep the unit tests as small as reasonably possible. For morphology tests, I did put a extras flag for pyLMeasure: https://github.com/scidash/neuronunit/blob/dev/setup.py#L27