Repository: aeye-lab/pymovements
Pull request #1088 (2025-04-11 23:12:12)

feat: add `DatasetDefinition.to_dict()`
## Description

Implements a `to_dict()` method for the `DatasetDefinition` class.

## Implemented changes

- [x] add `DatasetDefinition.to_dict()`
- [x] add `hide_private` argument to hide fields that start with `_`
- [x] resuse `to_dict()` in `to_yaml()`

## How Has This Been Tested?

- [x] test expected dicts for dataset definition instances
- [x] test expected dicts for definitions with private attributes

## Context

Resolves 

#### required by:
- #1064 

#### requires:
- 

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules
- [x] I have checked my code and corrected any misspellings


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/aeye-lab/pymovements/pull/1088?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.94%. Comparing base [(`259b768`)](https://app.codecov.io/gh/aeye-lab/pymovements/commit/259b76838d6f5115755de192eda1ba6d39f24ec1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) to head [(`12c6e22`)](https://app.codecov.io/gh/aeye-lab/pymovements/commit/12c6e22e338b480f77b5e29557a3e3cbd1fb7ca2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##              main    #1088      +/-   ##
===========================================
- Coverage   100.00%   99.94%   -0.06%     
===========================================
  Files           87       87              
  Lines         3717     3727      +10     
  Branches       638      641       +3     
===========================================
+ Hits          3717     3725       +8     
- Misses           0        1       +1     
- Partials         0        1       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/aeye-lab/pymovements/pull/1088?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
dkrako: The output of `to_dict()` currently includes a lot of attributes that are just `None` or empty dictionaries. I created #1091 to fix this.
dkrako: @SiQube just a small reminder that this PR is ready for review

Patch:
diff --git a/src/pymovements/dataset/_utils/_yaml.py b/src/pymovements/dataset/_utils/_yaml.py
index 4ccd296..3579d58 100644
--- a/src/pymovements/dataset/_utils/_yaml.py
+++ b/src/pymovements/dataset/_utils/_yaml.py
@@ -20,6 +20,10 @@
 """Yaml utilities."""
 from __future__ import annotations
 
+import builtins
+import importlib
+from typing import Any
+
 import yaml
 
 
@@ -92,3 +96,32 @@ def type_constructor(
         ) from exc
     except ValueError as exc:
         raise ValueError(f'Unknown {node=}') from exc
+
+
+def substitute_types(data: Any) -> Any:
+    """Substitute types in data with !-prefixed strings."""
+    if isinstance(data, dict):
+        return {k: substitute_types(v) for k, v in data.items()}
+    if isinstance(data, list):
+        return [substitute_types(v) for v in data]
+    if isinstance(data, type):
+        if data.__module__ == 'builtins':
+            return f'!{data.__name__}'
+        return f'!{data.__module__}.{data.__name__}'
+    return data
+
+
+def reverse_substitute_types(data: Any) -> Any:
+    """Substitute !-prefixed strings with python types."""
+    if isinstance(data, dict):
+        return {k: reverse_substitute_types(v) for k, v in data.items()}
+    if isinstance(data, list):
+        return [reverse_substitute_types(v) for v in data]
+    if isinstance(data, str) and data.startswith('!'):
+        type_name = data[1:]
+        if '.' in type_name:
+            module_name, class_name = type_name.rsplit('.', 1)
+            module = importlib.import_module(module_name)
+            return getattr(module, class_name)
+        return getattr(builtins, type_name)
+    return data
diff --git a/src/pymovements/dataset/dataset_definition.py b/src/pymovements/dataset/dataset_definition.py
index 8063c25..274b82d 100644
--- a/src/pymovements/dataset/dataset_definition.py
+++ b/src/pymovements/dataset/dataset_definition.py
@@ -20,8 +20,6 @@
 """DatasetDefinition module."""
 from __future__ import annotations
 
-import builtins
-import importlib
 from dataclasses import asdict
 from dataclasses import dataclass
 from dataclasses import field
@@ -30,6 +28,8 @@ from typing import Any
 
 import yaml
 
+from pymovements.dataset._utils._yaml import reverse_substitute_types
+from pymovements.dataset._utils._yaml import substitute_types
 from pymovements.dataset._utils._yaml import type_constructor
 from pymovements.gaze.experiment import Experiment
 from pymovements.gaze.eyetracker import EyeTracker
@@ -204,47 +204,47 @@ class DatasetDefinition:
                 eyetracker=eyetracker,
             )
 
-        def reverse_substitute_types(d: Any) -> Any:
-            if isinstance(d, dict):
-                return {k: reverse_substitute_types(v) for k, v in d.items()}
-            if isinstance(d, list):
-                return [reverse_substitute_types(v) for v in d]
-            if isinstance(d, str) and d.startswith('!'):
-                type_name = d[1:]
-                if '.' in type_name:
-                    module_name, class_name = type_name.rsplit('.', 1)
-                    module = importlib.import_module(module_name)
-                    return getattr(module, class_name)
-                return getattr(builtins, type_name)
-            return d
-
         data = reverse_substitute_types(data)
         # Initialize DatasetDefinition with YAML data
         return DatasetDefinition(**data)
 
-    def to_yaml(self, path: str | Path) -> None:
-        """Save a dataset definition to a YAML file.
+    def to_dict(self, exclude_private: bool = True) -> dict[str, Any]:
+        """Return dictionary representation.
 
         Parameters
         ----------
-        path: str | Path
-            Path where to save the YAML file to.
+        exclude_private: bool
+            Exclude attributes that start with `_`.
+
+        Returns
+        -------
+        dict[str, Any]
+            Dictionary representation of dataset definition.
         """
         data = asdict(self)
 
-        def substitute_types(d: Any) -> Any:
-            if isinstance(d, dict):
-                return {k: substitute_types(v) for k, v in d.items()}
-            if isinstance(d, list):
-                return [substitute_types(v) for v in d]
-            if isinstance(d, type):
-                if d.__module__ == 'builtins':
-                    return f'!{d.__name__}'
-                return f'!{d.__module__}.{d.__name__}'
-            return d
+        # Delete private fields from dictionary.
+        if exclude_private:
+            for key in list(data.keys()):
+                if key.startswith('_'):
+                    del data[key]
 
         data['experiment'] = data['experiment'].to_dict()
 
+        return data
+
+    def to_yaml(self, path: str | Path, exclude_private: bool = True) -> None:
+        """Save a dataset definition to a YAML file.
+
+        Parameters
+        ----------
+        path: str | Path
+            Path where to save the YAML file to.
+        exclude_private: bool
+            Exclude attributes that start with `_`.
+        """
+        data = self.to_dict(exclude_private=exclude_private)
+
         data = substitute_types(data)
 
         with open(path, 'w', encoding='utf-8') as f:


Test patch:
diff --git a/tests/unit/dataset/dataset_definition_test.py b/tests/unit/dataset/dataset_definition_test.py
index 0309ce2..f03ecbf 100644
--- a/tests/unit/dataset/dataset_definition_test.py
+++ b/tests/unit/dataset/dataset_definition_test.py
@@ -18,9 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 """Test dataset definition."""
-from dataclasses import asdict
 from dataclasses import dataclass
-from dataclasses import field
 
 import pytest
 import yaml
@@ -50,65 +48,279 @@ def test_dataset_definition_is_equal(init_kwargs):
     assert definition1 == definition2
 
 
-def test_dataset_definition_to_yaml_equal_dicts_no_exp(tmp_path):
-    tmp_file = tmp_path / 'tmp.yaml'
+@pytest.mark.parametrize(
+    ('definition', 'expected_dict'),
+    [
+        pytest.param(
+            DatasetDefinition(
+                name='Example',
+                has_files={
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+            ),
+            {
+                'name': 'Example',
+                'has_files': {
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+                'acceleration_columns': None,
+                'column_map': {},
+                'custom_read_kwargs': {},
+                'distance_column': None,
+                'experiment': {
+                    'eyetracker': {
+                        'left': None,
+                        'model': None,
+                        'mount': None,
+                        'right': None,
+                        'sampling_rate': None,
+                        'vendor': None,
+                        'version': None,
+                    },
+                    'screen': {
+                        'distance_cm': None,
+                        'height_cm': None,
+                        'height_px': None,
+                        'origin': 'upper left',
+                        'width_cm': None,
+                        'width_px': None,
+                    },
+                },
+                'extract': {},
+                'filename_format': {},
+                'filename_format_schema_overrides': {},
+                'mirrors': {},
+                'pixel_columns': None,
+                'position_columns': None,
+                'resources': {},
+                'time_column': None,
+                'time_unit': 'ms',
+                'trial_columns': None,
+                'velocity_columns': None,
+            },
+            id='no_experiment',
+        ),
 
-    @dataclass
-    class TestDatasetDefinition(DatasetDefinition):
-        name: str = 'Example'
-        has_files: dict[str, bool] = field(
-            default_factory=lambda: {
-                'gaze': False,
-                'precomputed_events': False,
-                'precomputed_reading_measures': False,
+        pytest.param(
+            DatasetDefinition(
+                name='Example',
+                has_files={
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+                experiment=Experiment(
+                    screen_width_px=1280,
+                    screen_height_px=1024,
+                    screen_width_cm=38.2,
+                    screen_height_cm=30.2,
+                    distance_cm=60,
+                    origin='center',
+                    sampling_rate=2000,
+                ),
+            ),
+            {
+                'name': 'Example',
+                'has_files': {
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+                'acceleration_columns': None,
+                'column_map': {},
+                'custom_read_kwargs': {},
+                'distance_column': None,
+                'experiment': {
+                    'eyetracker': {
+                        'left': None,
+                        'model': None,
+                        'mount': None,
+                        'right': None,
+                        'sampling_rate': 2000,
+                        'vendor': None,
+                        'version': None,
+                    },
+                    'screen': {
+                        'distance_cm': 60,
+                        'height_cm': 30.2,
+                        'height_px': 1024,
+                        'origin': 'center',
+                        'width_cm': 38.2,
+                        'width_px': 1280,
+                    },
+                },
+                'extract': {},
+                'filename_format': {},
+                'filename_format_schema_overrides': {},
+                'mirrors': {},
+                'pixel_columns': None,
+                'position_columns': None,
+                'resources': {},
+                'time_column': None,
+                'time_unit': 'ms',
+                'trial_columns': None,
+                'velocity_columns': None,
             },
-        )
+            id='experiment',
+        ),
+    ],
+)
+def test_dataset_definition_to_dict_expected(definition, expected_dict):
+    assert definition.to_dict() == expected_dict
 
-    definition = TestDatasetDefinition()
-    definition.to_yaml(tmp_file)
 
-    with open(tmp_file, encoding='utf-8') as f:
-        yaml_dict = yaml.safe_load(f)
+@pytest.mark.parametrize(
+    ('exclude_private', 'expected_dict'),
+    [
+        pytest.param(
+            True,
+            {
+                'name': 'MyDatasetDefinition',
+                'has_files': {},
+                'acceleration_columns': None,
+                'column_map': {},
+                'custom_read_kwargs': {},
+                'distance_column': None,
+                'experiment': {
+                    'eyetracker': {
+                        'left': None,
+                        'model': None,
+                        'mount': None,
+                        'right': None,
+                        'sampling_rate': None,
+                        'vendor': None,
+                        'version': None,
+                    },
+                    'screen': {
+                        'distance_cm': None,
+                        'height_cm': None,
+                        'height_px': None,
+                        'origin': 'upper left',
+                        'width_cm': None,
+                        'width_px': None,
+                    },
+                },
+                'extract': {},
+                'filename_format': {},
+                'filename_format_schema_overrides': {},
+                'mirrors': {},
+                'pixel_columns': None,
+                'position_columns': None,
+                'resources': {},
+                'time_column': None,
+                'time_unit': 'ms',
+                'trial_columns': None,
+                'velocity_columns': None,
+            },
+            id='True',
+        ),
 
-    definition_dict = asdict(definition)
-    definition_dict['experiment'] = definition_dict['experiment'].to_dict()
-    assert definition_dict == yaml_dict
+        pytest.param(
+            False,
+            {
+                'name': 'MyDatasetDefinition',
+                '_foobar': 'test',
+                'has_files': {},
+                'acceleration_columns': None,
+                'column_map': {},
+                'custom_read_kwargs': {},
+                'distance_column': None,
+                'experiment': {
+                    'eyetracker': {
+                        'left': None,
+                        'model': None,
+                        'mount': None,
+                        'right': None,
+                        'sampling_rate': None,
+                        'vendor': None,
+                        'version': None,
+                    },
+                    'screen': {
+                        'distance_cm': None,
+                        'height_cm': None,
+                        'height_px': None,
+                        'origin': 'upper left',
+                        'width_cm': None,
+                        'width_px': None,
+                    },
+                },
+                'extract': {},
+                'filename_format': {},
+                'filename_format_schema_overrides': {},
+                'mirrors': {},
+                'pixel_columns': None,
+                'position_columns': None,
+                'resources': {},
+                'time_column': None,
+                'time_unit': 'ms',
+                'trial_columns': None,
+                'velocity_columns': None,
+            },
+            id='False',
+        ),
+    ],
+)
+def test_dataset_definition_to_dict_exclude_private_expected(exclude_private, expected_dict):
+    @dataclass
+    class MyDatasetDefinition(DatasetDefinition):
+        name: str = 'MyDatasetDefinition'
+        _foobar: str = 'test'
 
+    definition = MyDatasetDefinition()
 
-def test_dataset_definition_to_yaml_equal_dicts(tmp_path):
-    tmp_file = tmp_path / 'tmp.yaml'
+    assert definition.to_dict(exclude_private=exclude_private) == expected_dict
 
-    @dataclass
-    class TestDatasetDefinition(DatasetDefinition):
-        name: str = 'Example'
-        has_files: dict[str, bool] = field(
-            default_factory=lambda: {
-                'gaze': False,
-                'precomputed_events': False,
-                'precomputed_reading_measures': False,
-            },
-        )
-        experiment: Experiment = field(
-            default_factory=lambda: Experiment(
-                screen_width_px=1280,
-                screen_height_px=1024,
-                screen_width_cm=38.2,
-                screen_height_cm=30.2,
-                distance_cm=60,
-                origin='center',
-                sampling_rate=2000,
+
+@pytest.mark.parametrize(
+    ('definition'),
+    [
+        pytest.param(
+            DatasetDefinition(
+                name='Example',
+                has_files={
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+            ),
+            id='no_exp',
+        ),
+
+        pytest.param(
+            DatasetDefinition(
+                name='Example',
+                has_files={
+                    'gaze': False,
+                    'precomputed_events': False,
+                    'precomputed_reading_measures': False,
+                },
+                experiment=Experiment(
+                    screen_width_px=1280,
+                    screen_height_px=1024,
+                    screen_width_cm=38.2,
+                    screen_height_cm=30.2,
+                    distance_cm=60,
+                    origin='center',
+                    sampling_rate=2000,
+                ),
             ),
-        )
+            id='no_exp',
+        ),
+    ],
+)
+def test_dataset_definition_to_yaml_equal_dicts(definition, tmp_path):
+    tmp_file = tmp_path / 'tmp.yaml'
 
-    definition = TestDatasetDefinition()
     definition.to_yaml(tmp_file)
 
     with open(tmp_file, encoding='utf-8') as f:
         yaml_dict = yaml.safe_load(f)
 
-    definition_dict = asdict(definition)
-    definition_dict['experiment'] = definition_dict['experiment'].to_dict()
-    assert definition_dict == yaml_dict
+    assert definition.to_dict() == yaml_dict
 
 
 def test_write_yaml_already_existing_dataset_definition_w_tuple_screen(tmp_path):
@@ -138,17 +350,13 @@ def test_check_equality_of_load_from_yaml_and_load_from_dictionary_dump(tmp_path
 
     yaml_definition = DatasetDefinition.from_yaml(dictionary_tmp_file)
 
-    @dataclass
-    class TestDatasetDefinition(DatasetDefinition):
-        name: str = 'Example'
-        has_files: dict[str, bool] = field(
-            default_factory=lambda: {
-                'gaze': False,
-                'precomputed_events': False,
-                'precomputed_reading_measures': False,
-            },
-        )
-
-    definition = TestDatasetDefinition()
+    expected_definition = DatasetDefinition(
+        name='Example',
+        has_files={
+            'gaze': False,
+            'precomputed_events': False,
+            'precomputed_reading_measures': False,
+        },
+    )
 
-    assert definition.__dict__ == yaml_definition.__dict__
+    assert yaml_definition == expected_definition
diff --git a/tests/unit/datasets/datasets_test.py b/tests/unit/datasets/datasets_test.py
index 012b554..f4f14d5 100644
--- a/tests/unit/datasets/datasets_test.py
+++ b/tests/unit/datasets/datasets_test.py
@@ -65,9 +65,7 @@ from pymovements import DatasetLibrary
 )
 def test_public_dataset_registered(definition, dataset_name):
     assert dataset_name in DatasetLibrary.names()
-    definition_from_library = DatasetLibrary.get(dataset_name).__dict__
-    python_definition = definition().__dict__
-    assert definition_from_library == python_definition
-    definition_from_lib_exp = definition_from_library.pop('experiment')
-    python_definition_exp = python_definition.pop('experiment')
-    assert python_definition_exp == definition_from_lib_exp
+    definition_from_library = DatasetLibrary.get(dataset_name)
+
+    # simple equal between objects does not work as classes have different names.
+    assert definition().to_dict() == definition_from_library.to_dict()