Repository: mlcommons/medperf (python)

Issue:
[BUG] Empty statistics from a data owner causes a global error
**Describe the bug**
The `generated_metadata` field of datasets in the client is set to be required:
https://github.com/mlcommons/medperf/blob/09edc70bcf7f1615c532c8f8b15f10de9b406e33/cli/medperf/entities/dataset.py#L30
However, on the server, it is allowed to be `None`.
We faced a data owner whose generated statistics file happened to be empty, thus registering the `generated_metadata` field in the server as `null`. This caused a field validation error on the client side for any user that uses `medperf dataset ls`.

**Additional context**
We need to make the `generated_metadata` field optional on the client side

Pull request:
prevent empty statistics file
Fixes #623

Patch:
diff --git a/cli/medperf/commands/dataset/prepare.py b/cli/medperf/commands/dataset/prepare.py
index 32ee6de..216d342 100644
--- a/cli/medperf/commands/dataset/prepare.py
+++ b/cli/medperf/commands/dataset/prepare.py
@@ -260,6 +260,13 @@ class DataPreparation:
             self.dataset.unmark_as_ready()
             raise e
 
+        with open(self.out_statistics_path) as f:
+            stats = yaml.safe_load(f)
+
+        if stats is None:
+            self.dataset.unmark_as_ready()
+            raise ExecutionError("Statistics file is empty.")
+
         self.ui.print("> Statistics complete")
 
     def mark_dataset_as_ready(self):
diff --git a/server/dataset/migrations/0005_alter_dataset_generated_metadata.py b/server/dataset/migrations/0005_alter_dataset_generated_metadata.py
new file mode 100644
index 0000000..1b5aafe
--- /dev/null
+++ b/server/dataset/migrations/0005_alter_dataset_generated_metadata.py
@@ -0,0 +1,18 @@
+# Generated by Django 4.2.11 on 2024-11-29 16:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("dataset", "0004_auto_20231211_1827"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="dataset",
+            name="generated_metadata",
+            field=models.JSONField(blank=True, default=dict),
+        ),
+    ]
diff --git a/server/dataset/models.py b/server/dataset/models.py
index 2af4c44..8b351c0 100644
--- a/server/dataset/models.py
+++ b/server/dataset/models.py
@@ -27,7 +27,7 @@ class Dataset(models.Model):
     state = models.CharField(
         choices=DATASET_STATE, max_length=100, default="DEVELOPMENT"
     )
-    generated_metadata = models.JSONField(default=dict, blank=True, null=True)
+    generated_metadata = models.JSONField(default=dict, blank=True)
     user_metadata = models.JSONField(default=dict, blank=True, null=True)
     report = models.JSONField(default=dict, blank=True, null=True)
     created_at = models.DateTimeField(auto_now_add=True)


Test patch:
diff --git a/cli/medperf/tests/commands/dataset/test_prepare.py b/cli/medperf/tests/commands/dataset/test_prepare.py
index e0e9ad6..dd09b03 100644
--- a/cli/medperf/tests/commands/dataset/test_prepare.py
+++ b/cli/medperf/tests/commands/dataset/test_prepare.py
@@ -56,7 +56,7 @@ def test_get_prep_cube_downloads_cube_file(mocker, data_preparation, cube):
 
 @pytest.mark.parametrize("dataset_for_test", [False, True])
 def test_prepare_with_test_data_doesnt_send_reports(
-        mocker, data_preparation, dataset_for_test, cube, comms, fs
+    mocker, data_preparation, dataset_for_test, cube, comms, fs
 ):
     # Arrange
     data_preparation.dataset.for_test = dataset_for_test
@@ -86,7 +86,9 @@ def test_prepare_runs_then_stops_report_handler(
     mocker.patch.object(data_preparation.dataset, "write")
     mocked_obs = mocker.create_autospec(spec=Observer)
     mocker.patch(PATCH_REGISTER.format("Observer"), side_effect=mocked_obs)
-    gen_report_spy = mocker.patch(PATCH_REGISTER.format("DataPreparation._DataPreparation__generate_report_dict"))
+    gen_report_spy = mocker.patch(
+        PATCH_REGISTER.format("DataPreparation._DataPreparation__generate_report_dict")
+    )
 
     # Act
     data_preparation.run_prepare()
@@ -208,11 +210,14 @@ def test_statistics_unmarks_the_dataset_as_ready_on_failure(
 
 @pytest.mark.parametrize("metadata_specified", [False, True])
 def test_statistics_checks_metadata_path(
-    mocker, data_preparation, metadata_specified, cube
+    mocker, data_preparation, metadata_specified, cube, fs
 ):
     # Arrange
     spy = mocker.patch.object(cube, "run")
     data_preparation.metadata_specified = metadata_specified
+    data_preparation.out_statistics_path = "test.yaml"
+    fs.create_file(data_preparation.out_statistics_path, contents="")
+    mocker.patch("yaml.safe_load", return_value={})
 
     # Act
     data_preparation.run_statistics()
@@ -224,6 +229,23 @@ def test_statistics_checks_metadata_path(
         assert "metadata_path" not in spy.call_args.kwargs.keys()
 
 
+def test_preparation_fails_if_statistics_is_none(mocker, data_preparation, cube, fs):
+
+    # Arrange
+    unmark_spy = mocker.patch.object(data_preparation.dataset, "unmark_as_ready")
+    mocker.patch.object(cube, "run")
+    data_preparation.out_statistics_path = "test.yaml"
+    fs.create_file(data_preparation.out_statistics_path, contents="")
+    mocker.patch("yaml.safe_load", return_value=None)
+
+    # Act
+    with pytest.raises(ExecutionError):
+        data_preparation.run_statistics()
+
+    # Assert
+    unmark_spy.assert_called_once()
+
+
 def test_dataset_is_updated_after_report_sending(mocker, data_preparation, comms):
     # Arrange
     send_spy = mocker.patch.object(comms, "update_dataset")