txt.py restructuring and acq read addition #123

vinferrer · 2020-01-10T09:16:15Z

Closes issue #137 and #136.
Add acq support mentioned in issue #41

Proposed Changes

restructuring populate_phys_input function so it only checks the header and recognizes the file type. File reading has been delegated to secondary functions. Added commented files support both in header and channels
process_labchart: new function that process labchart header and channels
process_acq: new function that process header and channels
read_header_and_channels: new function that reads txt files and separates them into header and channels
still missing multi-frequency support

…abchart_read

phys2bids/interfaces/txt.py

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

Delete unecessary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

smoia

Looks very promising!
It needs a couple of changes but it's looking good.

One thing: can you add (or collaborate with another developer to add) testing for this new file?
We should start not merging PRs without tests (docs aside). You can ask @eurunuela or @rmarkello for help in the task!

phys2bids/interfaces/txt.py

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

Co-Authored-By: Stefano Moia <[email protected]>

…abchart_read

…ex1 to be more orientative

smoia

Ok, we're getting there! A couple of comments and docstring reviews more, and I think we'll be ready!

phys2bids/interfaces/txt.py

smoia · 2020-01-13T11:19:32Z

phys2bids/interfaces/txt.py

+    # get units and names
+    orig_units = []
+    orig_names = []
+    for index1 in range(3, len(header[-1]) + 8, 2):


I think that @eurunuela 's point is still not met. Can you maybe comment this line to make it more clear (so we can help you with a name for the variable?)

so basically the variable is an index than used into the header retreats for every channel the original name and units

we could call it ``retriving_indx```?

phys2bids/interfaces/txt.py

…abchart_read

codecov-io · 2020-01-29T11:08:53Z

Codecov Report

❗ No coverage uploaded for pull request base (master@dd8c0eb). Click here to learn what that means.
The diff coverage is 97.05%.

@@           Coverage Diff            @@
##             master    #123   +/-   ##
========================================
  Coverage          ?   66.7%           
========================================
  Files             ?      11           
  Lines             ?     934           
  Branches          ?       0           
========================================
  Hits              ?     623           
  Misses            ?     311           
  Partials          ?       0

Impacted Files	Coverage Δ
phys2bids/utils.py	`91.17% <ø> (ø)`
phys2bids/tests/test_txt.py	`100% <100%> (ø)`
phys2bids/phys2bids.py	`37.5% <100%> (ø)`
phys2bids/cli/run.py	`19.23% <50%> (ø)`
phys2bids/viz.py	`15.09% <50%> (ø)`
phys2bids/physio_obj.py	`82.95% <88.88%> (ø)`

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 dd8c0eb...5c0a7cd. Read the comment docs.

eurunuela

Looks pretty good to me overall, just little details we should probably tackle before merging.

phys2bids/interfaces/txt.py

phys2bids/tests/test_txt.py

vinferrer · 2020-01-30T13:03:34Z

@smoia please review

vinferrer · 2020-01-30T15:36:43Z

Anddddd Travis is working now, I think you can accept the pull request safely @smoia, @eurunuela

smoia

Great example of how the PRs should be from now on!
There's only one little issue with the files. Change that and you're good to go!

phys2bids/tests/data/Test_belt_pulse_samefreq.txt

vinferrer · 2020-02-05T10:40:26Z

Changes requested done, please accept the review, @smoia @eurunuela

eurunuela

LGTM!

smoia

🎉 🎉 🎉

vinferrer added 6 commits December 19, 2019 16:13

define functions for each class of txt file, starting with labchart

eeb2328

adding support for commented files

3988045

adding raised error for unsupported formats

6d82ae0

new acq_read function

fb0db10

Merge branch 'master' of https://github.com/physiopy/phys2bids into l…

12c2b66

…abchart_read

acq_read function ready for pull request

482490f

vinferrer added Enhancement New feature or request Refactoring Improve nonfunctional attributes Minormod This PR generally closes an `Enhancement` issue. It increments the minor version (0.+1.0) labels Jan 10, 2020

vinferrer requested a review from smoia January 10, 2020 09:16

eurunuela reviewed Jan 10, 2020

View reviewed changes

vinferrer and others added 3 commits January 10, 2020 10:33

Update phys2bids/interfaces/txt.py

cc13e6f

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

Update phys2bids/interfaces/txt.py

665bb22

Delete unecessary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

Update phys2bids/interfaces/txt.py

b96ae58

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

smoia requested changes Jan 10, 2020

View reviewed changes

phys2bids/interfaces/txt.py Outdated Show resolved Hide resolved

phys2bids/interfaces/txt.py Outdated Show resolved Hide resolved

phys2bids/interfaces/txt.py Outdated Show resolved Hide resolved

phys2bids/interfaces/txt.py Outdated Show resolved Hide resolved

vinferrer and others added 8 commits January 10, 2020 12:03

Update phys2bids/interfaces/txt.py

d6c12f3

delete unnecesary f string Co-Authored-By: Eneko Uruñuela <[email protected]>

Update phys2bids/interfaces/txt.py

2418d1b

Co-Authored-By: Stefano Moia <[email protected]>

Merge branch 'master' of https://github.com/physiopy/phys2bids into l…

6ef4622

…abchart_read

Merge branch 'labchart_read' of github.com:vinferrer/phys2bids into l…

65629d2

…abchart_read

update txt.py docstrings to txt.py

97ee35c

chanching comment detection condition and changing kj variable to ind…

2f4db10

…ex1 to be more orientative

change functions order

3e13c3e

correct linting problems

f3f91af

vinferrer requested a review from smoia January 13, 2020 11:51

smoia requested changes Jan 13, 2020

View reviewed changes

vinferrer added 4 commits January 17, 2020 11:38

completting docstring and adding comments

24df90f

correct acq unit condition and comment detection problem

283dfc8

Merge branch 'master' of https://github.com/physiopy/phys2bids into l…

55fce55

…abchart_read

changing raise error to raise warning

dd8c0eb

vinferrer requested a review from smoia January 17, 2020 11:07

adding the possibility of frequency in the header instead of interval

753496c

sepparating header and channel list reading in a different function

afd2d57

eurunuela mentioned this pull request Jan 29, 2020

Add unit tests for txt.py #136

Closed

vinferrer added 3 commits January 30, 2020 12:13

wrong identation and numeric errors

b00b813

adding comment and space

b3bc3fb

adding unitest for txt.py

8939761

eurunuela requested changes Jan 30, 2020

View reviewed changes

phys2bids/interfaces/txt.py Outdated Show resolved Hide resolved

phys2bids/interfaces/txt.py Show resolved Hide resolved

phys2bids/tests/test_txt.py Show resolved Hide resolved

phys2bids/tests/test_txt.py Outdated Show resolved Hide resolved

linting problems

6593b82

vinferrer added 6 commits January 30, 2020 15:00

changes functions names and add docstring to read_header_channels

4e20342

adding test_raises function

096ff64

change import so it works in travis

1fbfd95

change travis to make it work

c6969ea

change import

00ee129

make travis work

2b3a4a0

smoia requested changes Feb 3, 2020

View reviewed changes

phys2bids/tests/data/Test_belt_pulse_samefreq.txt Show resolved Hide resolved

vinferrer mentioned this pull request Feb 5, 2020

Channel plot has incorrect units #137

Closed

vinferrer added 3 commits February 5, 2020 11:26

delete added comment to restore file

f70676b

adding file with comment on it

a3a7b5c

modify test_txt.py to include file with comment already on it

5c0a7cd

eurunuela approved these changes Feb 5, 2020

View reviewed changes

eurunuela mentioned this pull request Feb 5, 2020

Error when there is no 'time' column (for labchart txt files) #138

Closed

smoia approved these changes Feb 5, 2020

View reviewed changes

vinferrer merged commit 88133df into physiopy:master Feb 5, 2020

vinferrer deleted the labchart_read branch February 5, 2020 11:45

smoia mentioned this pull request Feb 5, 2020

Need to know which sampling interval units uses AcqKnowlegde format #129

Closed

smoia added the released This issue/pull request has been released. label Oct 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txt.py restructuring and acq read addition #123

txt.py restructuring and acq read addition #123

vinferrer commented Jan 10, 2020 •

edited

Loading

smoia left a comment •

edited

Loading

smoia left a comment •

edited

Loading

smoia Jan 13, 2020

vinferrer Jan 13, 2020

vinferrer Jan 13, 2020

codecov-io commented Jan 29, 2020 •

edited

Loading

eurunuela left a comment

vinferrer commented Jan 30, 2020

vinferrer commented Jan 30, 2020

smoia left a comment

vinferrer commented Feb 5, 2020

eurunuela left a comment

smoia left a comment

txt.py restructuring and acq read addition #123

txt.py restructuring and acq read addition #123

Conversation

vinferrer commented Jan 10, 2020 • edited Loading

Proposed Changes

smoia left a comment • edited Loading

Choose a reason for hiding this comment

smoia left a comment • edited Loading

Choose a reason for hiding this comment

smoia Jan 13, 2020

Choose a reason for hiding this comment

vinferrer Jan 13, 2020

Choose a reason for hiding this comment

vinferrer Jan 13, 2020

Choose a reason for hiding this comment

codecov-io commented Jan 29, 2020 • edited Loading

Codecov Report

eurunuela left a comment

Choose a reason for hiding this comment

vinferrer commented Jan 30, 2020

vinferrer commented Jan 30, 2020

smoia left a comment

Choose a reason for hiding this comment

vinferrer commented Feb 5, 2020

eurunuela left a comment

Choose a reason for hiding this comment

smoia left a comment

Choose a reason for hiding this comment

vinferrer commented Jan 10, 2020 •

edited

Loading

smoia left a comment •

edited

Loading

smoia left a comment •

edited

Loading

codecov-io commented Jan 29, 2020 •

edited

Loading