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 GE scanner physiological file functionality #424

Merged
merged 49 commits into from
Dec 23, 2022

Conversation

nw-duncan
Copy link
Contributor

@nw-duncan nw-duncan commented Apr 23, 2022

Functionality to read and process GE scanner physiological recording files is added to the tool.

Users can enter a single scanner generated file (with the name given to it by the scanner). Note that these do not have any suffix. If there are other files corresponding to other measurement modalities (i.e., PPG, respiration belt, or EKG) then these will be found automatically and added.

Proposed Changes

  • New function to check if an input file has the filename convention of a GE file. If so all other relevant files are found and a ".gep" suffix appended to each (preserving the processing pipeline implemented for other supported file types)
  • New function to read in the relevant files. This also adds a simulated trigger channel as GE scanners do not, to my knowledge, output such information natively
  • All other steps proceed as with currently supported files

Change Type

  • bugfix (+0.0.1)
  • minor (+0.1.0)
  • major (+1.0.0)
  • refactoring (no version update)
  • test (no version update)
  • infrastructure (no version update)
  • documentation (no version update)
  • Internal
  • Testing

Checklist before review

  • I added everything I wanted to add to this PR.
  • [Code or tests only] I wrote/updated the necessary docstrings.
  • [Code or tests only] I ran and passed tests locally.
  • [Documentation only] I built the docs locally.
  • My contribution is harmonious with the rest of the code: I'm not introducing repetitions.
  • My code respects the adopted style, especially linting conventions.
  • The title of this PR is explanatory on its own, enough to be understood as part of a changelog.
  • I added or indicated the right labels.
  • I added information regarding the timeline of completion for this PR.
  • Please, comment on my PR while it's a draft and give me feedback on the development!

@smoia smoia added the Minormod This PR generally closes an `Enhancement` issue. It increments the minor version (0.+1.0) label Apr 23, 2022
@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #424 (635cb56) into master (8185967) will increase coverage by 0.04%.
The diff coverage is 90.10%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #424      +/-   ##
==========================================
+ Coverage   94.77%   94.82%   +0.04%     
==========================================
  Files           8        8              
  Lines         881      966      +85     
==========================================
+ Hits          835      916      +81     
- Misses         46       50       +4     
Impacted Files Coverage Δ
phys2bids/io.py 96.63% <82.50%> (-3.37%) ⬇️
phys2bids/utils.py 98.09% <95.23%> (-0.35%) ⬇️
phys2bids/phys2bids.py 90.10% <100.00%> (+2.46%) ⬆️
phys2bids/physio_obj.py 94.03% <100.00%> (ø)

@smoia
Copy link
Member

smoia commented Nov 29, 2022

@nw-duncan once #430 is tested and merged, could you rebase this PR on the new master? I switched back CI to Circle CI, so we can exclude missed runs due to CI providers.

nw-duncan and others added 27 commits November 29, 2022 12:02
…nner physiological file. If so, adds a .gep filename extension.
…nner physiological file. If so, adds a .gep filename extension. Do nothing if extension is already present.
…input for trigger_idx. Make this input optional with default of 0. If "None" is set as the input then it records this in the log
…s an optional input in their BlueprintInput calls (need to follow up with this so that it's truly optional? At the moment it's hardcoded that they all will actually have an input)
…as visualisation is based upon there being a trigger channel. Changed load_gep to insert a column of zeros for the trigger channel
…rches for other modalities based on the filename the user enters. Need to modify check_ge in io.py before proceeding
… files and adds the .gep extension to them too
… files and adds the .gep extension to them too
…les that might be produced if phys2bids is run without a heuristics file or no subject id. Also stopped it adding .gep to files that already end in that
…lename contains data that isn't consistent with GE physiological format. Catches strings and files with more than one column
…ta where the scan is running (i.e., from 30s into the data to the end)
@smoia smoia marked this pull request as ready for review December 23, 2022 09:51
Copy link
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

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

LGTM!

@smoia
Copy link
Member

smoia commented Dec 23, 2022

@nw-duncan if you agree, we merge this part in now, and we wait for documentation later?

@nw-duncan
Copy link
Contributor Author

Yes, let's do that.

@smoia smoia merged commit 4464c84 into physiopy:master Dec 23, 2022
@smoia
Copy link
Member

smoia commented Dec 23, 2022

🚀 PR was released in 2.8.0 🚀

@smoia smoia added the released This issue/pull request has been released. label Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minormod This PR generally closes an `Enhancement` issue. It increments the minor version (0.+1.0) released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants