From 1324c5afef73e3df29baa584b14d300b587498e9 Mon Sep 17 00:00:00 2001 From: Murat Bilgel Date: Thu, 12 Sep 2024 18:03:30 -0400 Subject: [PATCH] remove kinfitr --- README.md | 17 --- docs/api.md | 9 +- poetry.lock | 49 +------- pyproject.toml | 5 - src/dynamicpet/__main__.py | 12 -- src/dynamicpet/kineticmodel/kinfitr.py | 147 ----------------------- tests/test_kineticmodel_simulated.py | 27 ----- tests/test_kinfitr.py | 155 ------------------------- tests/test_main.py | 37 ------ 9 files changed, 3 insertions(+), 455 deletions(-) delete mode 100644 src/dynamicpet/kineticmodel/kinfitr.py delete mode 100644 tests/test_kinfitr.py diff --git a/README.md b/README.md index 461e5fc..534761a 100644 --- a/README.md +++ b/README.md @@ -53,11 +53,6 @@ _Dynamic PET_ requires Python 3.11+ and the following modules: `nibabel` for neuroimaging data I/O and simple manipulations, and `click` for the CLI. -To run the `kinfitr` implementations of kinetic models, you will also need an -installation of R and the [kinfitr] R package as well as the `rpy2` Python module. - -[kinfitr]: https://github.com/mathesong/kinfitr - ## Installation You can install _Dynamic PET_ via [pip] after cloning the repository: @@ -67,18 +62,6 @@ $ git clone https://github.com/bilgelm/dynamicpet.git $ pip install -e dynamicpet ``` -If you would like to use kinetic models implemented in `kinfitr` with -_Dynamic PET_, first [install R](https://cran.r-project.org) -and the [kinfitr] R package. -Then install _Dynamic PET_ with the `kinfitr` extra after cloning the `git` -repository as descibed above: - -```console -$ pip install -e dynamicpet[kinfitr] -``` - -[kinfitr]: https://github.com/mathesong/kinfitr - ## Usage Please see the [Usage] for details on the CLI. diff --git a/docs/api.md b/docs/api.md index 63e8d37..7e51d50 100644 --- a/docs/api.md +++ b/docs/api.md @@ -47,7 +47,8 @@ ## dynamicpet.kineticmodel -These kinetic model implementations use the entire temporal extent of the TAC object provided for modeling. If you'd like to use a specific time interval for +These kinetic model implementations use the entire temporal extent of the TAC +object provided for modeling. If you'd like to use a specific time interval for modeling, first extract this time interval and then fit the kinetic model. ```{eval-rst} @@ -61,9 +62,3 @@ modeling, first extract this time interval and then fit the kinetic model. :members: :inherited-members: ``` - -```{eval-rst} -.. automodule:: dynamicpet.kineticmodel.kinfitr - :members: - :inherited-members: -``` diff --git a/poetry.lock b/poetry.lock index 6033a21..276a9fd 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3270,33 +3270,6 @@ files = [ {file = "rpds_py-0.20.0.tar.gz", hash = "sha256:d72a210824facfdaf8768cf2d7ca25a042c30320b3020de2fa04640920d4e121"}, ] -[[package]] -name = "rpy2" -version = "3.5.16" -description = "Python interface to the R language (embedded R)" -optional = true -python-versions = ">=3.7" -files = [ - {file = "rpy2-3.5.16-cp310-cp310-macosx_11_0_x86_64.whl", hash = "sha256:c748fc74ba01a51f6aca0a5f9e7bf637cd09413ffa031dd79b49b7a9fe97770b"}, - {file = "rpy2-3.5.16-cp311-cp311-macosx_10_9_universal2.whl", hash = "sha256:f076b34bd79f62ae583e75acc1b305ba73a6639ea5c9a44dc53896709ccd8ba0"}, - {file = "rpy2-3.5.16-cp38-cp38-macosx_11_0_x86_64.whl", hash = "sha256:3bb396851710856c6544c4278988b2abfe01d5a392278e5157b97148e62079c4"}, - {file = "rpy2-3.5.16-cp39-cp39-macosx_11_0_x86_64.whl", hash = "sha256:c067769dbade7faccdc8d2181ae5e29329fe0e66289fb291436a546da2f5e881"}, - {file = "rpy2-3.5.16.tar.gz", hash = "sha256:837e2f74583658a5c4c339761a73f9434f33ef9ced3e30c64da7562165c2801b"}, -] - -[package.dependencies] -cffi = ">=1.15.1" -jinja2 = "*" -packaging = {version = "*", markers = "platform_system == \"Windows\""} -tzlocal = "*" - -[package.extras] -all = ["ipython", "numpy", "pandas (>=1.3.5)", "pytest"] -pandas = ["numpy", "pandas (>=1.3.5)"] -test = ["ipython", "numpy", "pandas (>=1.3.5)", "pytest"] -test-minimal = ["coverage", "pytest (>=8)", "pytest-cov"] -types = ["mypy", "types-tzlocal"] - [[package]] name = "ruamel-yaml" version = "0.18.6" @@ -4090,23 +4063,6 @@ files = [ {file = "tzdata-2024.1.tar.gz", hash = "sha256:2674120f8d891909751c38abcdfd386ac0a5a1127954fbc332af6b5ceae07efd"}, ] -[[package]] -name = "tzlocal" -version = "5.2" -description = "tzinfo object for the local timezone" -optional = true -python-versions = ">=3.8" -files = [ - {file = "tzlocal-5.2-py3-none-any.whl", hash = "sha256:49816ef2fe65ea8ac19d19aa7a1ae0551c834303d5014c6d5a62e4cbda8047b8"}, - {file = "tzlocal-5.2.tar.gz", hash = "sha256:8d399205578f1a9342816409cc1e46a93ebd5755e39ea2d85334bea911bf0e6e"}, -] - -[package.dependencies] -tzdata = {version = "*", markers = "platform_system == \"Windows\""} - -[package.extras] -devenv = ["check-manifest", "pytest (>=4.3)", "pytest-cov", "pytest-mock (>=3.3)", "zest.releaser"] - [[package]] name = "urllib3" version = "2.2.3" @@ -4413,10 +4369,7 @@ enabler = ["pytest-enabler (>=2.2)"] test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", "jaraco.test", "more-itertools", "pytest (>=6,!=8.1.*)", "pytest-ignore-flaky"] type = ["pytest-mypy"] -[extras] -kinfitr = ["rpy2"] - [metadata] lock-version = "2.0" python-versions = ">=3.11,<3.13" -content-hash = "b908efa38fa479709c83ec41cff35fb8898c9a6694e6ec284b7aa9def90f84e8" +content-hash = "42cd804dd00266ccb23f0eb5455f6e0c02a047760b66a6f49d44b8d657f3f99c" diff --git a/pyproject.toml b/pyproject.toml index 4821515..119f00d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,13 +22,8 @@ numpy = ">=1.24.3" click = ">=8.1.3" scipy = ">=1.10.1" tqdm = ">=4.65.0" - -rpy2 = { version = ">=3.5.12", optional = true } types-tqdm = "^4.66.0.20240417" -[tool.poetry.extras] -kinfitr = ["rpy2"] - [tool.poetry.scripts] kineticmodel = "dynamicpet.__main__:kineticmodel" denoise = "dynamicpet.__main__:denoise" diff --git a/src/dynamicpet/__main__.py b/src/dynamicpet/__main__.py index 3a91280..daab838 100644 --- a/src/dynamicpet/__main__.py +++ b/src/dynamicpet/__main__.py @@ -30,8 +30,6 @@ "SUVR", "SRTMLammertsma1996", "SRTMZhou2003", - "kinfitr.SRTM", - "kinfitr.SRTM2", ] INTEGRATION_TYPES = ( @@ -281,16 +279,6 @@ def kineticmodel( weight_by=weight_by, fwhm=fwhm, ) - case "kinfitr.srtm": - from dynamicpet.kineticmodel import kinfitr - - km = kinfitr.SRTM(reftac, pet_img) - km.fit(mask=petmask_img_mat) - case "kinfitr.srtm2": - from dynamicpet.kineticmodel import kinfitr - - km = kinfitr.SRTM2(reftac, pet_img) - km.fit(mask=petmask_img_mat) case _: raise ValueError(f"Model {model} is not supported") diff --git a/src/dynamicpet/kineticmodel/kinfitr.py b/src/dynamicpet/kineticmodel/kinfitr.py deleted file mode 100644 index 76efb43..0000000 --- a/src/dynamicpet/kineticmodel/kinfitr.py +++ /dev/null @@ -1,147 +0,0 @@ -"""Wrapper for kinfitr.""" - -from abc import ABC - -import numpy as np -from rpy2.robjects import default_converter # type: ignore -from rpy2.robjects import numpy2ri -from rpy2.robjects import r -from rpy2.robjects.packages import importr # type: ignore -from tqdm import trange - -from ..temporalobject.temporalimage import TemporalImage -from ..temporalobject.temporalmatrix import TemporalMatrix -from ..typing_utils import NumpyNumberArray -from .kineticmodel import KineticModel - - -np_cv_rules = default_converter + numpy2ri.converter - -importr("kinfitr") - - -class KinfitrModel(KineticModel, ABC): - """Generic wrapper for kinfitr reference tissue models.""" - - @classmethod - def get_r_name(cls) -> str: - """Get the R function name.""" - return cls.__name__.lower() - - @classmethod - def get_param_names(cls) -> list[str]: - """Get names of kinetic model parameters.""" - return ["bp", "R1", "k2"] - - def fit( - self, mask: NumpyNumberArray | None = None, **kwargs: NumpyNumberArray | float - ) -> None: - """Estimate model parameters. - - Args: - mask: [optional] A 1-D (for TemporalMatrix TACs) or - 3-D (for TemporalImage TACs) binary mask that defines where - to fit the kinetic model. Elements outside the mask will - be set to to 0 in parametric estimate outputs. - kwargs: optional arguments for the kinfitr function - """ - tacs: TemporalMatrix = self.tacs.timeseries_in_mask(mask) - num_elements = tacs.num_elements - t_tac = self.reftac.frame_mid.flatten() - reftac = self.reftac.dataobj.flatten() - roitacs = tacs.dataobj.reshape(num_elements, tacs.num_frames) - - param_estimates = {} - - with np_cv_rules.context(): - for i in trange(num_elements): - kinfitr_fun = r[self.__class__.get_r_name()] - res = kinfitr_fun(t_tac, reftac, roitacs[i, :].flatten(), **kwargs) - for param_name in res["par"].dtype.names: - if param_name not in param_estimates: - param_estimates.update( - {param_name: np.zeros((num_elements, 1))} - ) - param_estimates[param_name][i] = res["par"][ # noqa: B909 - param_name - ] - - for param_name, param_estimate in param_estimates.items(): - self.set_parameter(param_name, param_estimate, mask) - - def fitted_tacs(self) -> TemporalMatrix | TemporalImage: - """Get fitted TACs based on estimated model parameters.""" - raise NotImplementedError() - - -class FRTM(KinfitrModel): - """kinfitr frtm wrapper.""" - - @classmethod - def get_param_names(cls) -> list[str]: - """Get names of kinetic model parameters.""" - return ["bp", "R1", "k2", "k3", "k4"] - - -class SRTM(KinfitrModel): - """kinfitr srtm wrapper.""" - - pass - - -class SRTM2(KinfitrModel): - """kinfitr srtm2 wrapper.""" - - @classmethod - def get_param_names(cls) -> list[str]: - """Get names of kinetic model parameters.""" - return ["bp", "R1", "k2", "k2a"] - - -class MRTM1(KinfitrModel): - """kinfitr mrtm1 wrapper.""" - - @classmethod - def get_param_names(cls) -> list[str]: - """Get names of kinetic model parameters.""" - return ["bp", "k2prime", "R1", "k2"] - - -class MRTM2(KinfitrModel): - """kinfitr mrtm2 wrapper.""" - - pass - - -class RefLogan(KinfitrModel): - """kinfitr refLogan wrapper.""" - - def __init__(self) -> None: - """Placeholder for raising error.""" - raise NotImplementedError("Doesn't work due to rpy2 error") - - @classmethod - def get_r_name(cls) -> str: - """Get the R function name.""" - return "refLogan" - - @classmethod - def get_param_names(cls) -> list[str]: - """Get names of kinetic model parameters.""" - return ["bp"] - - -class RefMLLogan(KinfitrModel): - """kinfitr refmlLogan wrapper.""" - - def __init__(self) -> None: - """Placeholder for raising error.""" - raise NotImplementedError() - - -class RefPatlak(KinfitrModel): - """kinfitr refPatlak wrapper.""" - - def __init__(self) -> None: - """Placeholder for raising error.""" - raise NotImplementedError() diff --git a/tests/test_kineticmodel_simulated.py b/tests/test_kineticmodel_simulated.py index 22f399c..4e40a71 100644 --- a/tests/test_kineticmodel_simulated.py +++ b/tests/test_kineticmodel_simulated.py @@ -268,30 +268,3 @@ def test_srtmlammertsma1996_tm( assert np.allclose(bp, bp_true, rtol=relative_tol) assert np.allclose(r1, r1_true, rtol=relative_tol) assert np.allclose(fitted_tacs.dataobj, tac.dataobj, rtol=relative_tol) - - -def test_srtmkinfitr_tm( - frame_start: NDArray[np.double], - frame_duration: NDArray[np.double], -) -> None: - """Test kinfitr SRTM wrapper.""" - kinfitr = pytest.importorskip("dynamicpet.kineticmodel.kinfitr") - - bp_true = 1.5 - r1_true = 1.2 - ct, cref = get_tacs_and_reftac_dataobj( - frame_start, frame_duration, bp_true, r1_true - ) - - tac = TemporalMatrix(ct, frame_start, frame_duration) - reftac = TemporalMatrix(cref, frame_start, frame_duration) - - km = kinfitr.SRTM(reftac, tac) - km.fit() - - bp: NumpyRealNumberArray = km.get_parameter("bp") - r1: NumpyRealNumberArray = km.get_parameter("R1") - - relative_tol = 0.006 - assert np.allclose(bp, bp_true, rtol=relative_tol) - assert np.allclose(r1, r1_true, rtol=relative_tol) diff --git a/tests/test_kinfitr.py b/tests/test_kinfitr.py deleted file mode 100644 index ee4f949..0000000 --- a/tests/test_kinfitr.py +++ /dev/null @@ -1,155 +0,0 @@ -"""Test cases for the kinfitr wrapper using data provided in kinfitr.""" - -from collections import namedtuple - -import numpy as np -import pytest - -from dynamicpet.kineticmodel.srtm import SRTMLammertsma1996 -from dynamicpet.kineticmodel.srtm import SRTMZhou2003 -from dynamicpet.temporalobject import TemporalMatrix - - -TACPair = namedtuple("TACPair", ["reftac", "tacs"]) -kinfitr = pytest.importorskip("dynamicpet.kineticmodel.kinfitr") -rpy2 = pytest.importorskip("rpy2") - - -@pytest.fixture -def simref0() -> TACPair: - """Get data for first subject in kinfitr's simref dataset.""" - kinfitr = rpy2.robjects.packages.importr("kinfitr") - simref = rpy2.robjects.packages.data(kinfitr).fetch("simref")["simref"] - # get data for participant at index i - i = 0 - simref_i = np.array(simref[3][i]) - - # Times, Reference, ROI1, ROI2, ROI3, Weights, StartTime, Duration - _, reftac, tac1, tac2, tac3, weights, frame_start, frame_duration = simref_i - - frame_start = frame_start - frame_duration = frame_duration - - # mitigate overlap issues by manipulating frame_duration - frame_duration = np.append(frame_start[1:] - frame_start[:-1], frame_duration[-1]) - - # drop first frame (which has 0 duration, so creates TemporalMatrix problems) - # this should not affect kinfitr functions as they will add the 0 back - reftac_tm = TemporalMatrix(reftac[1:], frame_start[1:], frame_duration[1:]) - tac_tm = TemporalMatrix( - np.vstack((tac1[1:], tac2[1:], tac3[1:])), - frame_start[1:], - frame_duration[1:], - ) - - return TACPair(reftac_tm, tac_tm) - - -def test_kinfitr_srtm(simref0: TACPair) -> None: - """Test kinfitr SRTM wrapper.""" - km = kinfitr.SRTM(simref0.reftac, simref0.tacs) - km.fit() - - # check that the results match those provided in kinfitr documentation - bp: float = km.get_parameter("bp")[0] - r1: float = km.get_parameter("R1")[0] - k2: float = km.get_parameter("k2")[0] - - assert np.round(bp, 6) == 1.488339 - assert np.round(r1, 6) == 1.233546 - assert np.round(k2, 6) == 0.101624 - - -def test_kinfitr_srtm2(simref0: TACPair) -> None: - """Test kinfitr SRTM2 wrapper.""" - km = kinfitr.SRTM2(simref0.reftac, simref0.tacs) - km.fit() - - km.get_parameter("bp") - km.get_parameter("k2a") - - -# def test_kinfitr_frtm(simref0: TACPair) -> None: -# """Test kinfitr SRTM2 wrapper.""" -# km = kinfitr.FRTM(simref0.reftac, simref0.tacs) -# km.fit() - -# km.get_parameter("bp") -# km.get_parameter("k3") - - -def test_kinfitr_mrtm1(simref0: TACPair) -> None: - """Test kinfitr MRTM1 wrapper using TemporalMatrix.""" - km = kinfitr.MRTM1(simref0.reftac, simref0.tacs) - km.fit(frameStartEnd=np.array([1, 20])) - - print(km.get_parameter("bp")) - print(km.get_parameter("k2prime")) - - -def test_kinfitr_mrtm2(simref0: TACPair) -> None: - """Test kinfitr MRTM2 wrapper.""" - km = kinfitr.MRTM2(simref0.reftac, simref0.tacs) - km.fit(k2prime=0.1, frameStartEnd=np.array([1, 20])) - - # check that the results match those provided in kinfitr documentation - bp: float = km.get_parameter("bp")[0] - k2: float = km.get_parameter("k2")[0] - - print(bp) - print(k2) - - # assert np.round(bp, 2) == round(1.488339, 2) - - -# def test_kinfitr_reflogan(simref0: TACPair) -> None: -# """Test kinfitr reflogan. DOESNT WORK; rpy2 error""" -# km = kinfitr.RefLogan(simref0.reftac, simref0.tacs) -# km.fit(k2prime=.1, tstarIncludedFrames=15) - -# print(km.get_parameter("bp")) - - -def test_srtm_zhou2003(simref0: TACPair) -> None: - """Test SRTM Zhou 2003.""" - km = SRTMZhou2003(simref0.reftac, simref0.tacs) - km.fit(integration_type="rect") - - # check that the results match those provided in kinfitr documentation - bp: float = km.get_parameter("bp")[0] # type: ignore - r1: float = km.get_parameter("r1")[0] # type: ignore - k2: float = km.get_parameter("k2")[0] # type: ignore - - assert np.round(bp, 2) == round(1.488339, 2) - assert np.round(r1, 1) == round(1.233546, 1) - assert np.round(k2, 1) == round(0.101624, 1) - - -def test_srtm_zhou2003_trapz(simref0: TACPair) -> None: - """Test SRTM Zhou 2003.""" - km = SRTMZhou2003(simref0.reftac, simref0.tacs) - km.fit(integration_type="trapz") - - # check that the results match those provided in kinfitr documentation - bp: float = km.get_parameter("bp")[0] # type: ignore - r1: float = km.get_parameter("r1")[0] # type: ignore - k2: float = km.get_parameter("k2")[0] # type: ignore - - assert np.round(bp, 1) == round(1.488339, 1) - assert np.round(r1, 1) == round(1.233546, 1) - assert np.round(k2, 1) == round(0.101624, 1) - - -def test_srtm_lammertsma1996(simref0: TACPair) -> None: - """Test SRTM Lammertsma 1996.""" - km = SRTMLammertsma1996(simref0.reftac, simref0.tacs) - km.fit() - - # check that the results match those provided in kinfitr documentation - bp: float = km.get_parameter("bp")[0] # type: ignore - r1: float = km.get_parameter("r1")[0] # type: ignore - k2: float = km.get_parameter("k2")[0] # type: ignore - - assert np.round(bp, 1) == round(1.488339, 1) - assert np.round(r1, 1) == round(1.233546, 1) - assert np.round(k2, 2) == round(0.101624, 2) diff --git a/tests/test_main.py b/tests/test_main.py index 899ea70..df1de48 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -329,40 +329,3 @@ def test_kineticmodel_srtmzhou2003(images: dict[str, Path]) -> None: assert result.exit_code == 0 assert os.path.isfile(outputdir / ("pet_km-" + model + "_kp-dvr.nii")) assert os.path.isfile(outputdir / ("pet_km-" + model + "_kp-r1.nii")) - - -def test_kineticmodel_kinfitr_srtm(images: dict[str, Path]) -> None: - """Test kinfitr's SRTM in __main__.py.""" - pytest.importorskip("dynamicpet.kineticmodel.kinfitr") - - from dynamicpet.__main__ import kineticmodel - - # first, test with tsv TACs - petjson_fname = images["petjson_fname"] - rois_csv_fname = images["rois_csv_fname"] - outputdir = rois_csv_fname.parent / "test_output" / "kinfitr_srtm" - - model = "kinfitr.srtm" - - runner = CliRunner() - result = runner.invoke( - kineticmodel, - [ - str(rois_csv_fname), - "--model", - model, - "--refroi", - "ROI1", - "--json", - str(petjson_fname), - "--start", - str(0), - "--end", - str(70), - "--outputdir", - str(outputdir), - ], - catch_exceptions=False, - ) - assert result.exit_code == 0 - assert os.path.isfile(outputdir / ("rois_km-" + model.replace(".", "") + ".tsv"))