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 BIDS reading support and prepare input loading for pydra workflow #7

Merged
merged 17 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"code",
"ideas",
"infra",
"review",
"review"
]
},
{
Expand All @@ -38,7 +38,7 @@
"data",
"ideas",
"infra",
"projectManagement",
"projectManagement"
]
},
{
Expand All @@ -51,6 +51,20 @@
"review",
"test"
]
},
{
"login": "maestroque",
"name": "George Kikas",
"avatar_url": "https://avatars.githubusercontent.com/u/74024609?v=4",
"profile": "https://github.com/maestroque",
"contributions": [
"code",
"ideas",
"infra",
"bug",
"test",
"review"
Copy link
Member

Choose a reason for hiding this comment

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

@maestroque please put yourself before Vicente (in alphabetical order of family name). You have all of the above, minus review, but plus documentation.
@me-pic and @m-miedema please add yourselves as well, mentoring and review.

]
}
],
"contributorsPerLine": 7,
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
<td align="center"><a href="http://rossmarkello.com"><img src="https://avatars0.githubusercontent.com/u/14265705?v=4" width="100px;" alt=""/><br /><sub><b>Ross Markello</b></sub></a><br /><a href="https://github.com/physiopy/phys2bids/commits?author=rmarkello" title="Code">💻</a> <a href="#ideas-rmarkello" title="Ideas, Planning, & Feedback">🤔</a> <a href="#infra-rmarkello" title="Infrastructure (Hosting, Build-Tools, etc)">🚇</a> <a href="https://github.com/physiopy/phys2bids/pulls?q=is%3Apr+reviewed-by%3Armarkello" title="Reviewed Pull Requests">👀</a></td>
<td align="center"><a href="https://github.com/smoia"><img src="https://avatars3.githubusercontent.com/u/35300580?v=4" width="100px;" alt=""/><br /><sub><b>Stefano Moia</b></sub></a><br /><a href="https://github.com/physiopy/phys2bids/commits?author=smoia" title="Code">💻</a> <a href="#data-smoia" title="Data">🔣</a> <a href="#ideas-smoia" title="Ideas, Planning, & Feedback">🤔</a> <a href="#infra-smoia" title="Infrastructure (Hosting, Build-Tools, etc)">🚇</a> <a href="#projectManagement-smoia" title="Project Management">📆</a></td>
<td align="center"><a href="https://github.com/eurunuela"><img src="https://avatars0.githubusercontent.com/u/13706448?v=4" width="100px;" alt=""/><br /><sub><b>Eneko Uruñuela</b></sub></a><br /><a href="https://github.com/physiopy/phys2bids/commits?author=eurunuela" title="Code">💻</a> <a href="https://github.com/physiopy/phys2bids/pulls?q=is%3Apr+reviewed-by%3Aeurunuela" title="Reviewed Pull Requests">👀</a> <a href="https://github.com/physiopy/phys2bids/commits?author=eurunuela" title="Tests">⚠️</a></td>
<td align="center"><a href="https://github.com/maestroque"><img src="https://avatars.githubusercontent.com/u/74024609?v=4?s=100" width="100px;" alt="George Kikas"/><br /><sub><b>George Kikas</b></sub></a><br /><a href="https://github.com/physiopy/phys2denoise/commits?author=maestroque" title="Code">💻</a> <a href="#ideas-maestroque" title="Ideas, Planning, & Feedback">🤔</a> <a href="#infra-maestroque" title="Infrastructure (Hosting, Build-Tools, etc)">🚇</a> <a href="https://github.com/physiopy/phys2denoise/issues?q=author%3Amaestroque" title="Bug reports">🐛</a> <a href="https://github.com/physiopy/phys2denoise/commits?author=maestroque" title="Tests">⚠️</a> <a href="https://github.com/physiopy/phys2denoise/pulls?q=is%3Apr+reviewed-by%3Amaestroque" title="Reviewed Pull Requests">👀</a></td>
Copy link
Member

Choose a reason for hiding this comment

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

Don't worry about this, I'll regenerate the table later

</tr>
</table>

Expand Down
9 changes: 7 additions & 2 deletions physutils/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import os.path as op

import numpy as np
from bids import BIDSLayout
from loguru import logger

from physutils import physio
Expand All @@ -28,7 +27,7 @@ def load_from_bids(
suffix="physio",
):
"""
Load physiological data from BIDS-formatted directory
Load physiological data from BIDS-formatted directory.
Parameters
----------
Expand All @@ -50,6 +49,12 @@ def load_from_bids(
data : :class:`physutils.Physio`
Loaded physiological data
"""
try:
from bids import BIDSLayout
except ImportError:
raise ImportError(
"To use BIDS-based feature, pybids must be installed. Install manually or with `pip install physutils[bids]`"
)

# check if file exists and is in BIDS format
if not op.exists(bids_path):
Expand Down
70 changes: 70 additions & 0 deletions physutils/tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

"""Helper class for holding physiological data and associated metadata information."""

import logging

from .io import load_from_bids, load_physio
from .physio import Physio
from .utils import is_bids_directory

# from loguru import logger

try:
from pydra import task
Copy link
Contributor

Choose a reason for hiding this comment

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

@smoia Is it possible that here it should be from pydra.mark import task instead of from pydra import task ?

Copy link
Contributor

Choose a reason for hiding this comment

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

With that change test_generate_physio_bids_file and test_generate_physio_auto in test_tasks.py are now passing 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Yes that could be it actually. Thank you for catching it!

except ImportError:
from .utils import task


LGR = logging.getLogger(__name__)
LGR.setLevel(logging.DEBUG)


@task
def generate_physio(
input_file: str, mode="auto", fs=None, bids_parameters=dict(), col_physio_type=None
) -> Physio:
"""
Load a physio object from either a BIDS directory or an exported physio object.

Parameters
----------
input_file : str
Path to input file
mode : 'auto', 'physio', or 'bids', optional
Mode to operate with
fs : None, optional
Set or force set sapmling frequency (Hz).
bids_parameters : dictionary, optional
Dictionary containing BIDS parameters
col_physio_type : int or None, optional
Object to pick up in a BIDS array of physio objects.

"""
LGR.info(f"Loading physio object from {input_file}")

if mode == "auto":
if input_file.endswith((".phys", ".physio", ".1D", ".txt", ".tsv", ".csv")):
mode = "physio"
elif is_bids_directory(input_file):
mode = "bids"
else:
raise ValueError(
"Could not determine input mode automatically. Please specify it manually."
)
if mode == "physio":
physio_obj = load_physio(input_file, fs=fs, allow_pickle=True)

elif mode == "bids":
if bids_parameters is {}:
raise ValueError("BIDS parameters must be provided when loading from BIDS")
else:
physio_array = load_from_bids(input_file, **bids_parameters)
physio_obj = (
physio_array[col_physio_type] if col_physio_type else physio_array
)
else:
raise ValueError(f"Invalid generate_physio mode: {mode}")

return physio_obj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"SamplingFrequency": 10000.0,
"StartTime": -3,
"Columns": [
"time",
"respiratory_chest",
"trigger",
"cardiac",
"respiratory_CO2",
"respiratory_O2"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"SamplingFrequency": 10000.0,
"StartTime": -3,
"Columns": [
"time",
"respiratory_chest",
"trigger",
"cardiac",
"respiratory_CO2",
"respiratory_O2"
]
}
107 changes: 107 additions & 0 deletions physutils/tests/test_tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
"""Tests for physutils.tasks and their integration."""

import os

import physutils.tasks as tasks
from physutils import physio
from physutils.tests.utils import create_random_bids_structure


def test_generate_physio_phys_file():
"""Test generate_physio task."""
physio_file = os.path.abspath("physutils/tests/data/ECG.phys")
task = tasks.generate_physio(input_file=physio_file, mode="physio")
assert task.inputs.input_file == physio_file
assert task.inputs.mode == "physio"
assert task.inputs.fs is None
Comment on lines +14 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

pytest is failing.. inputs is not an attribute included in the Physio class, nor input_file or mode...

Copy link
Contributor Author

@maestroque maestroque Oct 16, 2024

Choose a reason for hiding this comment

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

inputs is a pydra task attribute specifying the input struct of the task, it may be that the new changes making pydra optional broke the tests. Before the tests were successful


task()
me-pic marked this conversation as resolved.
Show resolved Hide resolved

physio_obj = task.result().output.out
assert isinstance(physio_obj, physio.Physio)
assert physio_obj.fs == 1000
assert physio_obj.data.shape == (44611,)


def test_generate_physio_bids_file():
"""Test generate_physio task."""
create_random_bids_structure("physutils/tests/data", recording_id="cardiac")
bids_parameters = {
"subject": "01",
"session": "01",
"task": "rest",
"run": "01",
"recording": "cardiac",
}
bids_dir = os.path.abspath("physutils/tests/data/bids-dir")
task = tasks.generate_physio(
input_file=bids_dir,
mode="bids",
bids_parameters=bids_parameters,
bids_channel="cardiac",
Copy link
Contributor

@me-pic me-pic Oct 16, 2024

Choose a reason for hiding this comment

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

This is failing since bids_channel is not a valid argument in the generate_physio function

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok just realized looking at the previous comments that bids_channel was changed to col_physio_type in generate_physio, but the test_tasks.py have not been update accordingly

)

assert task.inputs.input_file == bids_dir
assert task.inputs.mode == "bids"
assert task.inputs.fs is None
assert task.inputs.bids_parameters == bids_parameters
assert task.inputs.bids_channel == "cardiac"

task()

physio_obj = task.result().output.out
assert isinstance(physio_obj, physio.Physio)


def test_generate_physio_auto():
create_random_bids_structure("physutils/tests/data", recording_id="cardiac")
bids_parameters = {
"subject": "01",
"session": "01",
"task": "rest",
"run": "01",
"recording": "cardiac",
}
bids_dir = os.path.abspath("physutils/tests/data/bids-dir")
task = tasks.generate_physio(
input_file=bids_dir,
mode="auto",
bids_parameters=bids_parameters,
bids_channel="cardiac",
me-pic marked this conversation as resolved.
Show resolved Hide resolved
)

assert task.inputs.input_file == bids_dir
assert task.inputs.mode == "auto"
assert task.inputs.fs is None
assert task.inputs.bids_parameters == bids_parameters
assert task.inputs.bids_channel == "cardiac"

task()

physio_obj = task.result().output.out
assert isinstance(physio_obj, physio.Physio)


def test_generate_physio_auto_error(caplog):
bids_dir = os.path.abspath("physutils/tests/data/non-bids-dir")
task = tasks.generate_physio(
input_file=bids_dir,
mode="auto",
bids_channel="cardiac",
)

assert task.inputs.input_file == bids_dir
assert task.inputs.mode == "auto"
assert task.inputs.fs is None
assert task.inputs.bids_channel == "cardiac"

try:
task()
except Exception:
assert caplog.text.count("ERROR") == 1
assert (
caplog.text.count(
"dataset_description.json' is missing from project root. Every valid BIDS dataset must have this file."
)
== 1
)
68 changes: 68 additions & 0 deletions physutils/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

"""Helper class for holding physiological data and associated metadata information."""

import logging
from functools import wraps

from loguru import logger

LGR = logging.getLogger(__name__)
LGR.setLevel(logging.DEBUG)


def task(func):
"""
Fake task decorator to import when pydra is not installed/used.

Parameters
----------
func: function
Function to run the wrapper around

Returns
-------
function
"""

@wraps(func)
def wrapper(*args, **kwargs):
return func(*args, **kwargs)
LGR.debug(
"Pydra is not installed, thus generate_physio is not available as a pydra task. Using the function directly"
)

return wrapper


def is_bids_directory(path_to_dir):
"""
Check if a directory is a BIDS compliant directory.

Parameters
----------
path_to_dir : os.path or str
Path to (supposed) BIDS directory

Returns
-------
bool
True if the given path is a BIDS directory, False is not.
"""
try:
from bids import BIDSLayout
except ImportError:
raise ImportError(
"To use BIDS-based feature, pybids must be installed. Install manually or with `pip install physutils[bids]`"
)
try:
# Attempt to create a BIDSLayout object
_ = BIDSLayout(path_to_dir)
return True
except Exception as e:
# Catch other exceptions that might indicate the directory isn't BIDS compliant
logger.error(
f"An error occurred while trying to load {path_to_dir} as a BIDS Layout object: {e}"
)
return False
10 changes: 7 additions & 3 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ classifiers =
License :: OSI Approved :: Apache Software License
Programming Language :: Python :: 3
license = Apache-2.0
description = Set of utilities meant to be used with Physiopy's libraries
description = Set of utilities meant to be used with Physiopy libraries
long_description = file:README.md
long_description_content_type = text/markdown; charset=UTF-8
platforms = OS Independent
Expand All @@ -23,9 +23,7 @@ python_requires = >=3.6.1
install_requires =
matplotlib
numpy >=1.9.3
scipy
loguru
pybids
tests_require =
pytest >=3.6
test_suite = pytest
Expand All @@ -34,6 +32,10 @@ packages = find:
include_package_data = True

[options.extras_require]
pydra =
pydra
bids =
pybids
doc =
sphinx >=2.0
sphinx-argparse
Expand All @@ -49,6 +51,8 @@ test =
scipy
pytest >=5.3
pytest-cov
%(pydra)s
%(bids)s
%(style)s
devtools =
pre-commit
Expand Down