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.

Sign up for GitHub

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

Jump to bottom

Quick fix to have a BIDS compliant output #187

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Quick fix to have a BIDS compliant output #187

merged 3 commits into from
Mar 26, 2020

Conversation

smoia
Copy link
Member

@smoia smoia commented Mar 26, 2020 •
edited
Loading

Closes #184

Proposed Changes

  • timeseries matrix gets transposed during its export, to be BIDS compliant.

(thanks to @sangfrois for pointing it out)

@smoia smoia added the BugFIX This PR generally closes a `Bug` issue, and increments the patch version (0.0.+1) label Mar 26, 2020
@codecov
Copy link

codecov bot commented Mar 26, 2020 •
edited
Loading

Codecov Report

Merging #187 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #187   +/-   ##
=======================================
  Coverage   94.54%   94.54%           
=======================================
  Files           7        7           
  Lines         568      568           
=======================================
  Hits          537      537           
  Misses         31       31
Impacted Files Coverage Δ
phys2bids/phys2bids.py 90.44% <100.00%> (ø)

rmarkello
rmarkello reviewed Mar 26, 2020
View reviewed changes
Copy link
Member

@rmarkello rmarkello left a comment

Choose a reason for hiding this comment

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

I think just using the .T on the timeseries array might be a bit more simple, in this case! What do you think?

phys2bids/phys2bids.py Outdated Show resolved Hide resolved
phys2bids/phys2bids.py Outdated Show resolved Hide resolved
Stefano Moia and others added 2 commits March 26, 2020 17:40
@rmarkello
Update phys2bids/phys2bids.py
a4bdef3 
Co-Authored-By: Ross Markello <rossmarkello@gmail.com>
@rmarkello
Update phys2bids/phys2bids.py
4fc05d5 
Co-Authored-By: Ross Markello <rossmarkello@gmail.com>
@smoia
Copy link
Member Author

smoia commented Mar 26, 2020

Sure thing!

@smoia
Copy link
Member Author

smoia commented Mar 26, 2020

@rmarkello when you agree, you can merge it in!

sangfrois
sangfrois approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member

@sangfrois sangfrois left a comment •
edited
Loading

Choose a reason for hiding this comment

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

simple and elegant! :)

I could've done it myself ; will be more proactive next time

@rmarkello rmarkello merged commit 9ec9c33 into physiopy:master Mar 26, 2020
@smoia smoia deleted the fix/output branch March 26, 2020 18:00
@smoia smoia mentioned this pull request Mar 26, 2020
Merged
@smoia smoia changed the title Quick fix for #184 Quick fix to have a BIDS compliant output Mar 26, 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
Reviewers

@rmarkello rmarkello rmarkello approved these changes

@sangfrois sangfrois sangfrois approved these changes

Assignees
No one assigned
Labels
BugFIX This PR generally closes a `Bug` issue, and increments the patch version (0.0.+1) released This issue/pull request has been released.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Output files are not BIDS compliant
3 participants
@smoia @rmarkello @sangfrois