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

Add unit tests for participants_file and append_list_as_row #254

Merged
merged 13 commits into from
Jun 19, 2020

Conversation

sangfrois
Copy link
Member

Adresses #252 by testing new functions in bids.py and utils.py

Proposed Changes

  • testing append_list_as_row()
  • testing participants_file()

This would lead to increased coverage 👍

@sangfrois sangfrois requested a review from eurunuela June 17, 2020 17:10
@sangfrois
Copy link
Member Author

I'm assigning Eneko to review this PR as we discussed.

No pressure. I still have to add the tests for the participants file.

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #254 into master will increase coverage by 0.98%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
+ Coverage   93.39%   94.38%   +0.98%     
==========================================
  Files           9        9              
  Lines         772      783      +11     
==========================================
+ Hits          721      739      +18     
+ Misses         51       44       -7     
Impacted Files Coverage Δ
phys2bids/phys2bids.py 90.97% <93.33%> (+0.81%) ⬆️
phys2bids/bids.py 96.11% <0.00%> (+6.79%) ⬆️

@sangfrois sangfrois changed the title adding test for row appending adding tests for bids participants file Jun 17, 2020
@smoia smoia added the Testing This is for testing features, writing tests or producing testing code label Jun 17, 2020
phys2bids/tests/test_utils.py Outdated Show resolved Hide resolved
phys2bids/tests/test_utils.py Outdated Show resolved Hide resolved
@sangfrois sangfrois marked this pull request as ready for review June 18, 2020 19:35
@sangfrois
Copy link
Member Author

Both new functions should be checked and up to date with master. I'm not sure about the last tests in test_bids.py - Waiting for feedback.

@eurunuela eurunuela changed the title adding tests for bids participants file Add unit tests for participants_file and append_list_as_row Jun 19, 2020
@eurunuela
Copy link
Collaborator

eurunuela commented Jun 19, 2020

@sangfrois I added the remaining test. It was just a matter of repeating the last one you wrote to check that the subject was already in the file.

I don't know why Travis is complaining about numpy today. It probably has something to do with the lowest numpy version we support. Anyway, the test pass and coverage increases significantly, so good job! 🙌🏻

Let's see if we fix the issue in Travis and then we review and merge this.

@eurunuela eurunuela requested a review from vinferrer June 19, 2020 08:17
Copy link
Collaborator

@vinferrer vinferrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice unit tests

@eurunuela eurunuela merged commit cd246b4 into physiopy:master Jun 19, 2020
@smoia smoia added the released This issue/pull request has been released. label Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released. Testing This is for testing features, writing tests or producing testing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants