From c45803530b6885deba6f8cb065b821fc9c0ca6fd Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Tue, 27 Jun 2023 14:29:27 +0530 Subject: [PATCH 1/9] [ENH]: Enable `stdout` and `stderr` capturing for `AFNI` commands (#234) * fix: enable stdout and stderr capture for fALFF via AFNI * fix: enable stdout and stderr capture for ReHo via AFNI * chore: add changelog 234.enh --- docs/changes/newsfragments/234.enh | 1 + junifer/markers/falff/falff_estimator.py | 5 ++--- junifer/markers/reho/reho_estimator.py | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 docs/changes/newsfragments/234.enh diff --git a/docs/changes/newsfragments/234.enh b/docs/changes/newsfragments/234.enh new file mode 100644 index 000000000..60eddbb85 --- /dev/null +++ b/docs/changes/newsfragments/234.enh @@ -0,0 +1 @@ +Enable ``stdout`` and ``stderr`` capture for AFNI commands by `Synchon Mandal`_ diff --git a/junifer/markers/falff/falff_estimator.py b/junifer/markers/falff/falff_estimator.py index 035a9eaca..f6bf57cae 100644 --- a/junifer/markers/falff/falff_estimator.py +++ b/junifer/markers/falff/falff_estimator.py @@ -70,12 +70,11 @@ class ALFFEstimator: If AFNI command fails. """ logger.info(f"AFNI command to be executed: {cmd}") - # TODO: Figure out how to capture stdout and stderr process = subprocess.run( cmd, stdin=subprocess.DEVNULL, - # stdout=subprocess.STDOUT, - # stderr=subprocess.STDOUT, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, shell=True, check=False, ) diff --git a/junifer/markers/reho/reho_estimator.py b/junifer/markers/reho/reho_estimator.py index 8d3d2802f..211c5d44c 100644 --- a/junifer/markers/reho/reho_estimator.py +++ b/junifer/markers/reho/reho_estimator.py @@ -173,6 +173,8 @@ class ReHoEstimator: reho_process = subprocess.run( reho_cmd_str, # string needed with shell=True stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, shell=True, # needed for respecting $PATH check=False, ) @@ -205,6 +207,8 @@ class ReHoEstimator: convert_process = subprocess.run( convert_cmd_str, # string needed with shell=True stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, shell=True, # needed for respecting $PATH check=False, ) -- 2.52.0 From 9fe6d8ee541980bbd8e75c63e6df1b6037b26398 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:17:23 +0200 Subject: [PATCH 2/9] update: add abstract method read() for BaseFeatureStorage --- junifer/storage/base.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/junifer/storage/base.py b/junifer/storage/base.py index 506758c1a..ab9c6eeda 100644 --- a/junifer/storage/base.py +++ b/junifer/storage/base.py @@ -104,6 +104,34 @@ class BaseFeatureStorage(ABC): klass=NotImplementedError, ) + @abstractmethod + def read( + self, + feature_name: Optional[str] = None, + feature_md5: Optional[str] = None, + ) -> Dict[ + str, Union[str, List[Union[int, str, Dict[str, str]]], np.ndarray] + ]: + """Read stored feature. + + Parameters + ---------- + feature_name : str, optional + Name of the feature to read (default None). + feature_md5 : str, optional + MD5 hash of the feature to read (default None). + + Returns + ------- + dict + The stored feature as a dictionary. + + """ + raise_error( + msg="Concrete classes need to implement read().", + klass=NotImplementedError, + ) + @abstractmethod def read_df( self, -- 2.52.0 From 6b1874e137069f7526c5f7041cb823a86a3f2f11 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:17:58 +0200 Subject: [PATCH 3/9] update: add test for BaseFeatureStorage.read() --- junifer/storage/tests/test_storage_base.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/junifer/storage/tests/test_storage_base.py b/junifer/storage/tests/test_storage_base.py index 9b75cc7bb..f08967705 100644 --- a/junifer/storage/tests/test_storage_base.py +++ b/junifer/storage/tests/test_storage_base.py @@ -38,6 +38,12 @@ def test_BaseFeatureStorage() -> None: def list_features(self): super().list_features() + def read(self, feature_name=None, feature_md5=None): + super().read( + feature_name=feature_name, + feature_md5=feature_md5, + ) + def read_df(self, feature_name=None, feature_md5=None): super().read_df( feature_name=feature_name, @@ -66,6 +72,9 @@ def test_BaseFeatureStorage() -> None: with pytest.raises(NotImplementedError): st.list_features() + with pytest.raises(NotImplementedError): + st.read(None) + with pytest.raises(NotImplementedError): st.read_df(None) -- 2.52.0 From 3715099fd39a9442d1470a7f437441d066e93a28 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:18:32 +0200 Subject: [PATCH 4/9] update: add not implemented read() method for SQLiteFeatureStorage --- junifer/storage/sqlite.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index adbcd3662..c1e1f9fda 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -235,6 +235,33 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): out[md5][k] = json.loads(v) return out + def read( + self, + feature_name: Optional[str] = None, + feature_md5: Optional[str] = None, + ) -> Dict[ + str, Union[str, List[Union[int, str, Dict[str, str]]], np.ndarray] + ]: + """Read stored feature. + + Parameters + ---------- + feature_name : str, optional + Name of the feature to read (default None). + feature_md5 : str, optional + MD5 hash of the feature to read (default None). + + Returns + ------- + dict + The stored feature as a dictionary. + + """ + raise_error( + msg="read() is not implemented for SQLiteFeatureStorage.", + klass=NotImplementedError, + ) + def read_df( self, feature_name: Optional[str] = None, -- 2.52.0 From 91174adf99609d60bd2f10c4070dcb5be8512051 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:19:06 +0200 Subject: [PATCH 5/9] update: add test for SQLiteFeatureStorage.read() --- junifer/storage/tests/test_sqlite.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/junifer/storage/tests/test_sqlite.py b/junifer/storage/tests/test_sqlite.py index 0310876f9..b347ff4a8 100644 --- a/junifer/storage/tests/test_sqlite.py +++ b/junifer/storage/tests/test_sqlite.py @@ -253,6 +253,22 @@ def test_upsert_invalid_option(tmp_path: Path) -> None: SQLiteFeatureStorage(uri=uri, upsert="wrong") +def test_read(tmp_path: Path) -> None: + """Test reading of stored feature. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + uri = tmp_path / "test_read.sqlite" + storage = SQLiteFeatureStorage(uri=uri, upsert="ignore") + # Check error + with pytest.raises(NotImplementedError): + storage.read() + + # TODO: can the tests be separated? def test_store_df_and_read_df(tmp_path: Path) -> None: """Test storing dataframe and reading of stored table into dataframe. -- 2.52.0 From fd8cf65fa136f37ad325469aa034755902df6661 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:23:45 +0200 Subject: [PATCH 6/9] update: add read() for HDF5FeatureStorage and adapt read_df() --- junifer/storage/hdf5.py | 70 +++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/junifer/storage/hdf5.py b/junifer/storage/hdf5.py index d9d7aa81a..2fdb38db2 100644 --- a/junifer/storage/hdf5.py +++ b/junifer/storage/hdf5.py @@ -328,15 +328,14 @@ class HDF5FeatureStorage(BaseFeatureStorage): return data - def read_df( + def read( self, feature_name: Optional[str] = None, - feature_md5: Optional[bool] = None, - ) -> pd.DataFrame: - """Read feature into a pandas.DataFrame. - - Either one of ``feature_name`` or ``feature_md5`` needs to be - specified. + feature_md5: Optional[str] = None, + ) -> Dict[ + str, Union[str, List[Union[int, str, Dict[str, str]]], np.ndarray] + ]: + """Read stored feature. Parameters ---------- @@ -347,8 +346,8 @@ class HDF5FeatureStorage(BaseFeatureStorage): Returns ------- - pandas.DataFrame - The features as a dataframe. + dict + The stored feature as a dictionary. Raises ------ @@ -427,10 +426,46 @@ class HDF5FeatureStorage(BaseFeatureStorage): title=md5, slash="ignore", ) + return hdf_data + + def read_df( + self, + feature_name: Optional[str] = None, + feature_md5: Optional[str] = None, + ) -> pd.DataFrame: + """Read feature into a pandas.DataFrame. + + Either one of ``feature_name`` or ``feature_md5`` needs to be + specified. + + Parameters + ---------- + feature_name : str, optional + Name of the feature to read (default None). + feature_md5 : str, optional + MD5 hash of the feature to read (default None). + + Returns + ------- + pandas.DataFrame + The features as a dataframe. + + Raises + ------ + IOError + If HDF5 file does not exist. + + """ + hdf_data = self.read( + feature_name=feature_name, feature_md5=feature_md5 + ) reshaped_data = hdf_data["data"] # Generate index for the data - logger.debug(f"Generating pandas.MultiIndex for {md5} ...") + logger.debug( + "Generating pandas.MultiIndex for feature " + f"{feature_name or feature_md5} ..." + ) if hdf_data["kind"] == "matrix": # Set index for element @@ -477,17 +512,26 @@ class HDF5FeatureStorage(BaseFeatureStorage): idx_df = pd.DataFrame(data=element_idx) # type: ignore # Create multiindex from dataframe hdf_data_idx = pd.MultiIndex.from_frame(df=idx_df) - logger.debug(f"Generated pandas.MultiIndex for {md5} ...") + logger.debug( + "Generated pandas.MultiIndex for feature " + f"{feature_name or feature_md5} ..." + ) # Convert to DataFrame - logger.debug(f"Converting HDF5 data for {md5} to pandas.DataFrame ...") + logger.debug( + "Converting HDF5 data for feature " + f"{feature_name or feature_md5} to pandas.DataFrame ..." + ) df = pd.DataFrame( data=reshaped_data, index=hdf_data_idx, columns=columns, # type: ignore dtype=reshaped_data.dtype, ) - logger.debug(f"Converted HDF5 data for {md5} to pandas.DataFrame ...") + logger.debug( + "Converted HDF5 data for feature " + f"{feature_name or feature_md5} to pandas.DataFrame ..." + ) return df -- 2.52.0 From c3cc6b1accaefe4d4a6c5dc82012353f992946c4 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 12:24:19 +0200 Subject: [PATCH 7/9] update: add test for HDF5FeatureStorage.read() --- junifer/storage/tests/test_hdf5.py | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/junifer/storage/tests/test_hdf5.py b/junifer/storage/tests/test_hdf5.py index 301e2f924..dda476003 100644 --- a/junifer/storage/tests/test_hdf5.py +++ b/junifer/storage/tests/test_hdf5.py @@ -220,6 +220,37 @@ def test_store_metadata_ignore_duplicate(tmp_path: Path) -> None: assert len(features) == 1 +def test_read(tmp_path: Path) -> None: + """Test read. + + Parameters + ---------- + tmp_path : pathlib.Path + The path to the test directory. + + """ + uri = tmp_path / "test_read.hdf5" + storage = HDF5FeatureStorage(uri=uri) + # Data to store + to_store_data = { + "meta": { + "element": {"subject": "test"}, + "dependencies": ["numpy"], + "marker": {"name": "test"}, + "type": "BOLD", + }, + "data": np.array([[1, 10]]), + "col_names": ["f1", "f2"], + } + # Store data + storage.store(kind="vector", **to_store_data) + # Read and check + stored_data = storage.read(feature_name="BOLD_test") + assert ["column_headers", "data", "element", "kind"] == list( + stored_data.keys() + ) + + def test_read_df_params_error(tmp_path: Path) -> None: """Test parameter validation errors for read_df. -- 2.52.0 From bd993e8cbf16bbb3131ce230f38beffb2992f793 Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Wed, 28 Jun 2023 15:08:09 +0200 Subject: [PATCH 8/9] chore: add changelogs 236.change and 236.enh --- docs/changes/newsfragments/236.change | 1 + docs/changes/newsfragments/236.enh | 1 + 2 files changed, 2 insertions(+) create mode 100644 docs/changes/newsfragments/236.change create mode 100644 docs/changes/newsfragments/236.enh diff --git a/docs/changes/newsfragments/236.change b/docs/changes/newsfragments/236.change new file mode 100644 index 000000000..83d746f0f --- /dev/null +++ b/docs/changes/newsfragments/236.change @@ -0,0 +1 @@ +Add :meth:`.BaseFeatureStorage.read` method for storage-like objects by `Synchon Mandal`_ diff --git a/docs/changes/newsfragments/236.enh b/docs/changes/newsfragments/236.enh new file mode 100644 index 000000000..86734a2eb --- /dev/null +++ b/docs/changes/newsfragments/236.enh @@ -0,0 +1 @@ +Adapt :meth:`.HDF5FeatureStorage.read_df` due to addition of :meth:`.HDF5FeatureStorage.read` by `Synchon Mandal`_ -- 2.52.0 From a8e565fd17c1891d62ead87f35a520601357ac2e Mon Sep 17 00:00:00 2001 From: Synchon Mandal Date: Mon, 3 Jul 2023 12:28:40 +0200 Subject: [PATCH 9/9] chore: improve message for unimplemented SQLiteFeatureStorage.read() --- junifer/storage/sqlite.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/junifer/storage/sqlite.py b/junifer/storage/sqlite.py index c1e1f9fda..101492420 100644 --- a/junifer/storage/sqlite.py +++ b/junifer/storage/sqlite.py @@ -258,7 +258,12 @@ class SQLiteFeatureStorage(PandasBaseFeatureStorage): """ raise_error( - msg="read() is not implemented for SQLiteFeatureStorage.", + msg=( + "read() for SQLiteFeatureStorage is not currently planned to " + "be implemented in the near future. If you need the " + "functionality, contact the junifer developers " + "(https://juaml.github.io/junifer/main/help.html)." + ), klass=NotImplementedError, ) -- 2.52.0