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

TST: Test core, and related changes #62

Merged
merged 19 commits into from
Mar 11, 2019

Conversation

scottclowe
Copy link
Member

  • Add a new function/script to downsample the example data and turn it into a compact set of small files for testing, fissa/tests/generate_downsampled_resources.py.
  • Generate test resources set, a, and add it to the repo.
  • Add unit tests for core.Experiment
  • Bug fix for deltaf with low number of samples
  • Use unittest's subTest context manager in test_deltaf
  • Add graceful degredation for subTest (which doesn't exist until Python 3.4) by providing an empty context manager when it isn't defined.
  • Bug fix for Basic usage notebook, which wasn't using datahandler_custom for the custom datahandler (was using datahandler instead, which was silently ignored by core.Experiment.__init__)

which uses these to generate test resources set `a`.
For Python <3.4, we provide a subTest context manager that does
nothing, so that there is graceful degredation of subTest.
If the data contains very few timepoints, our 30 taps mean the
automatic padlen used by filtfilt of `3 * ntaps` will exceed the
length of the data. We prevent this error by specifying the padlen
to the min of `3 * ntaps` and one less than the length of the data,
however further work needs to be done to give a better methodology
when the dataset is small.
Previously, the notebook example was
```
fissa.Experiment(..., datahandler=datahandler_custom)
```
which doesn't actually supply the correct argument.
The correct argument is
```
fissa.Experiment(..., datahandler_custom=datahandler_custom)
```
This was not caught previously because the Experiment class had
an unused `**kwargs` keyword parameter collection, which was accepting
the erroneous `datahandler` argument. As the `datahandler_custom`
example is the same as `fissa.datahandler`, the fact that the argument
was ignored was not noticed.
@codecov
Copy link

codecov bot commented Mar 11, 2019

Codecov Report

Merging #62 into master will increase coverage by 40.73%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #62       +/-   ##
===========================================
+ Coverage   38.44%   79.18%   +40.73%     
===========================================
  Files           9        9               
  Lines         658      663        +5     
  Branches      128      129        +1     
===========================================
+ Hits          253      525      +272     
+ Misses        371       94      -277     
- Partials       34       44       +10
Impacted Files Coverage Δ
fissa/roitools.py 80.86% <0%> (+59.63%) ⬆️
fissa/deltaf.py 100% <100%> (+12.5%) ⬆️
fissa/core.py 90.14% <100%> (+79.71%) ⬆️
fissa/neuropil.py 67.56% <0%> (+9.45%) ⬆️
fissa/datahandler.py 85.18% <0%> (+25.92%) ⬆️
fissa/datahandler_framebyframe.py 88.23% <0%> (+47.05%) ⬆️

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 751ee19...6517775. Read the comment docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 130

  • 2 of 4 (50.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+22.2%) to 62.391%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fissa/roitools.py 0 2 0.0%
Totals Coverage Status
Change from base Build 129: 22.2%
Covered Lines: 433
Relevant Lines: 661

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 11, 2019

Pull Request Test Coverage Report for Build 133

  • 5 of 6 (83.33%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+42.7%) to 82.882%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fissa/roitools.py 1 2 50.0%
Totals Coverage Status
Change from base Build 129: 42.7%
Covered Lines: 569
Relevant Lines: 663

💛 - Coveralls

Usually, `pool.join()` is needed to ensure the parent process is
blocked until the pool is completed, however `pool.map` also
blocks, so we didn't need it for that.
https://docs.python.org/2/library/multiprocessing.html#multiprocessing.Process.join

However, `pool.join` ensures the pool is cleaned up properly and
exceptions are caught correctly.
https://stackoverflow.com/a/38271957/1960959

For us, we need to use `pool.join` so the coverage is measured
during each of the processes which are spawned, and they are
amalgamated together afterwards.
https://pytest-cov.readthedocs.io/en/latest/mp.html#ungraceful-pool-shutdown
In case two sets of unit tests are run at the same millisecond,
or tests are run in parallel.
@scottclowe scottclowe merged commit 15664ea into rochefort-lab:master Mar 11, 2019
@scottclowe scottclowe mentioned this pull request Jul 10, 2019
@scottclowe scottclowe deleted the tst_test-core2 branch March 8, 2020 19:00
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

Successfully merging this pull request may close these issues.

2 participants