Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move unit testing datasets elsewhere #216

Open
rgerkin opened this issue Apr 20, 2019 · 4 comments
Open

Move unit testing datasets elsewhere #216

rgerkin opened this issue Apr 20, 2019 · 4 comments

Comments

@rgerkin
Copy link
Contributor

rgerkin commented Apr 20, 2019

@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
Copy link
Collaborator

JustasB commented Apr 21, 2019

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: Travis fix #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
Copy link
Collaborator

JustasB commented Apr 21, 2019

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
Copy link
Contributor Author

rgerkin commented Apr 23, 2019

@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
Copy link
Collaborator

JustasB commented Apr 23, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants