Repository: aeon-toolkit/aeon
Pull request #740 (2023-09-11 01:25:40)

[BUG] fixes multivariate dataframe tsfile writes (#732)
#### Reference Issues/PRs
https://github.com/aeon-toolkit/aeon/issues/732

#### What does this implement/fix? Explain your changes.

Added appropriate parameters for the dataframe ts file writer to output the proper header.  Also one minor fix to `write_to_tsfile()` (see comment below).  

#### Did you add any tests for the change?

Yes in `aeon/datasets/tests/test_data_writers.py` I added `test_write_dataframe_to_ts_and_load`.  However, this test is currently failing, see comment below.

#### Any other comments?

* In `write_to_tsfile()` the initial check was just calling `isinstance(pd.Dataframe)` which I switched to `isinstance(X, pd.DataFrame)`.  It should have raised at some point though but maybe that function has never been called before without X being a `list` or `numpy.ndarray`? 
* I don't expect my current test to pass because the behavior of `load_from_tsfile()` hasn't been changed in this PR.  I wasn't sure if the idea was to keep `load_from_tsfile()` and `load_from_tsfile_to_dataframe()` as explicitly separate operations or if a user writing a dataframe to a a ts file should expect `load_from_tsfile()` to also return them a dataframe (right now it's np.ndarray).  I would personally expect a dataframe if I've written a dataframe out but I'm not sure whether that's the intention and if so how to store that information within the header (or elsewhere?) so the loader can be made aware.  Maybe a `@datatype` in the header?   

### PR checklist

<!--
Please go through the checklist below. Please feel free to remove points if they are not applicable.
-->

##### For all contributions
- [x] I've added myself to the [list of contributors](https://github.com/aeon-toolkit/aeon/blob/main/.all-contributorsrc).
- [ ] Optionally, I've updated aeon's [CODEOWNERS](https://github.com/aeon-toolkit/aeon/blob/main/CODEOWNERS) to receive notifications about future changes to these files.
- [X] The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.
- [ ] For new estimators, I've added the estimator to the online documentation.


<!--
Thanks for contributing!
-->


TonyBagnall: hi, thanks for this, its very welcome. The dataframe loaders/writers are legacy really, back to the time when we stored collections in nested dataframes when each cell was a pd.Series. Good spot on the isinstance check, also found it here when looking to improve coverage. https://github.com/aeon-toolkit/aeon/pull/733. I am happy to leave 733 and update from yours once we get it in. I'll take a look today.
TonyBagnall:  >I wasn't sure if the idea was to keep load_from_tsfile() and load_from_tsfile_to_dataframe()

hi @hedeershowk, just to clarify whats going on here, because its not necessarily intuitive and is a bit of a legacy to the origins of aeon  ....

load_from_tsfile() just loads data into either numpy (if series are equal length) or list of numpy (if series are unequal). These are the standard formats for the machine learning side (i.e. not forecasting): for equal length series stored as numpy is shape (n_cases, n_channels, n_timepoints). This structure is not amenable to standard pd.DataFrames, which in the simplest case for equal length with n_channels == 1 would store in shape (n_cases, n_timepoints) or (n_timepoints, n_cases), and if n_channels > 1 it gets complicated. You can use hierarchical/long formats,  but its messy.

Originally, some contributors really wanted to use data frames, so overcame these problems by storing data in a pd.DataFrame shape (n_cases, n_channels), with each entry a time series of length n_timepoints. This non standard usage tended to confuse everyone, so we have more recently attempted to implicitly deprecate it by removing usage. The x_to_dataframe functions are a legacy feature to load and read in this format. It means in recent refactoring I tried to ignore them, but you have found me out :)

So back to your PR. ITs useful, thanks, and good spot with the bug. I'd very much like to encourage your participation, we try to be as welcoming as possible. I will try take a look at how it fits in tomorrow, but I have quite a lot on, sorry if it takes me a little longer. 

If you want to  discuss further, please join our slack, as in #general or #contributors, or direct message me.    


  
hedeershowk: >  >I wasn't sure if the idea was to keep load_from_tsfile() and load_from_tsfile_to_dataframe()
> 
> hi @hedeershowk, just to clarify whats going on here, because its not necessarily intuitive and is a bit of a legacy to the origins of aeon  ....
> 
> load_from_tsfile() just loads data into either numpy (if series are equal length) or list of numpy (if series are unequal). These are the standard formats for the machine learning side (i.e. not forecasting): for equal length series stored as numpy is shape (n_cases, n_channels, n_timepoints). This structure is not amenable to standard pd.DataFrames, which in the simplest case for equal length with n_channels == 1 would store in shape (n_cases, n_timepoints) or (n_timepoints, n_cases), and if n_channels > 1 it gets complicated. You can use hierarchical/long formats,  but its messy.
> 
> Originally, some contributors really wanted to use data frames, so overcame these problems by storing data in a pd.DataFrame shape (n_cases, n_channels), with each entry a time series of length n_timepoints. This non standard usage tended to confuse everyone, so we have more recently attempted to implicitly deprecate it by removing usage. The x_to_dataframe functions are a legacy feature to load and read in this format. It means in recent refactoring I tried to ignore them, but you have found me out :)
> 
> So back to your PR. ITs useful, thanks, and good spot with the bug. I'd very much like to encourage your participation, we try to be as welcoming as possible. I will try take a look at how it fits in tomorrow, but I have quite a lot on, sorry if it takes me a little longer. 
> 
> If you want to  discuss further, please join our slack, as in #general or #contributors, or direct message me.    
> 
> 
>   

Ah okay thanks for the background information.  Makes sense and I can appreciate the desire to get things into data frames (in some cases) and the complexity of getting that done properly with the shape. 

I could update the test to more explicitly test the header construction the way you did in the bug report or just use the dataframe loader.  No rush here, just let me know what you'd prefer to get the PR how it can be most useful to you.

I've also joined the Slack (thanks) and look forward to participating and contributing more.
hedeershowk: cleaned up that test and I switched to [tempfile](https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory).  I was actually running into a bunch of file not found errors when running tests in this directory (even in the main branch).  Not sure why just running `pytest` from the module root didn't fail but when I explicitly ran pytest on `test_data_writers.py` I had a bunch of fails.  This PR fixes that. 
hedeershowk: hm haven't dug into it but looks like maybe the test fail in `test_proportion_train_in_param_finding()` is related to the shortcut in base when dealing with a single class?  I'll try to reproduce and resolve it locally.
hedeershowk: Not clear how my changes could have an impact on the current failing test in CI?  I'll run locally to confirm and see if I can figure out the issue as well.
TonyBagnall: @hedeershowk could you update this with main (and resolve contributors conflict)? I think a lot of these changes were also in your other PR. Thanks, Tony


Patch:
diff --git a/.github/ISSUE_TEMPLATE/sweep-fast-template.yml b/.github/ISSUE_TEMPLATE/sweep-fast-template.yml
new file mode 100644
index 00000000..ff4aab15
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/sweep-fast-template.yml
@@ -0,0 +1,14 @@
+name: Sweep Fast Issue
+title: 'Sweep (fast): '
+description: For few-line fixes to be handled by Sweep, an AI-powered junior developer. Sweep will use GPT-3.5 to quickly create a PR for very small changes.
+labels: sweep
+body:
+  - type: textarea
+    id: description
+    attributes:
+      label: Details
+      description: Tell Sweep where and what to edit and provide enough context for a new developer to the codebase
+      placeholder: |
+        Bugs: The bug might be in ... file. Here are the logs: ...
+        Features: the new endpoint should use the ... class from ... file because it contains ... logic.
+        Refactors: We are migrating this function to ... version because ...
diff --git a/.github/ISSUE_TEMPLATE/sweep-slow-template.yml b/.github/ISSUE_TEMPLATE/sweep-slow-template.yml
new file mode 100644
index 00000000..1f238402
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/sweep-slow-template.yml
@@ -0,0 +1,14 @@
+name: Sweep Slow Issue
+title: 'Sweep (slow): '
+description: For larger bugs, features, refactors, and tests to be handled by Sweep, an AI-powered junior developer. Sweep will perform a deeper search and more self-reviews but will take longer.
+labels: sweep
+body:
+  - type: textarea
+    id: description
+    attributes:
+      label: Details
+      description: Tell Sweep where and what to edit and provide enough context for a new developer to the codebase
+      placeholder: |
+        Bugs: The bug might be in ... file. Here are the logs: ...
+        Features: the new endpoint should use the ... class from ... file because it contains ... logic.
+        Refactors: We are migrating this function to ... version because ...
diff --git a/.github/ISSUE_TEMPLATE/sweep-template.yml b/.github/ISSUE_TEMPLATE/sweep-template.yml
new file mode 100644
index 00000000..98811de1
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/sweep-template.yml
@@ -0,0 +1,14 @@
+name: Sweep Issue
+title: 'Sweep: '
+description: For small bugs, features, refactors, and tests to be handled by Sweep, an AI-powered junior developer.
+labels: sweep
+body:
+  - type: textarea
+    id: description
+    attributes:
+      label: Details
+      description: Tell Sweep where and what to edit and provide enough context for a new developer to the codebase
+      placeholder: |
+        Bugs: The bug might be in ... file. Here are the logs: ...
+        Features: the new endpoint should use the ... class from ... file because it contains ... logic.
+        Refactors: We are migrating this function to ... version because ...
diff --git a/aeon/datasets/_data_writers.py b/aeon/datasets/_data_writers.py
index dd01e7e0..d5d69617 100644
--- a/aeon/datasets/_data_writers.py
+++ b/aeon/datasets/_data_writers.py
@@ -37,7 +37,7 @@ def write_to_tsfile(
         the header since there is no definite way of inferring this from y
     """
     if not (
-        isinstance(X, np.ndarray) or isinstance(X, list) or isinstance(pd.DataFrame)
+        isinstance(X, np.ndarray) or isinstance(X, list) or isinstance(X, pd.DataFrame)
     ):
         raise TypeError(
             f" Wrong input data type {type(X)} convert to np.ndarray ("
@@ -57,8 +57,8 @@ def write_to_tsfile(
             path,
             problem_name=problem_name,
             y=y,
-            equal_length=False,
             comment=header,
+            regression=regression,
         )
 
 
@@ -332,11 +332,7 @@ def _write_header(
 
 
 def _write_dataframe_to_tsfile(
-    X,
-    path,
-    problem_name="sample_data",
-    y=None,
-    comment=None,
+    X, path, problem_name="sample_data", y=None, comment=None, regression=False
 ):
     # ensure data provided is a dataframe
     if not isinstance(X, pd.DataFrame):
@@ -348,11 +344,20 @@ def _write_dataframe_to_tsfile(
     class_labels = None
     if y is not None:
         class_labels = np.unique(y)
+    univariate = X.shape[1] == 1
+    # dataframes are always equal length
+    equal_length = True
+    series_length = X.shape[0]
     file = _write_header(
         path,
         problem_name,
-        comment=comment,
+        univariate=univariate,
+        equal_length=equal_length,
+        series_length=series_length,
         class_labels=class_labels,
+        comment=comment,
+        regression=regression,
+        extension=None,
     )
     n_cases, n_dimensions = X.shape
     for i in range(0, n_cases):
diff --git a/sweep.yaml b/sweep.yaml
new file mode 100644
index 00000000..4f98c727
--- /dev/null
+++ b/sweep.yaml
@@ -0,0 +1,12 @@
+# Sweep AI turns bug fixes & feature requests into code changes (https://sweep.dev)
+# For details on our config file, check out our docs at https://docs.sweep.dev
+
+# If you use this be sure to frequently sync your default branch(main, master) to dev.
+branch: 'main'
+# By default Sweep will read the logs and outputs from your existing Github Actions. To disable this, set this to false.
+gha_enabled: True
+# This is the description of your project. It will be used by sweep when creating PRs. You can tell Sweep what's unique about your project, what frameworks you use, or anything else you want.
+# Here's an example: sweepai/sweep is a python project. The main api endpoints are in sweepai/api.py. Write code that adheres to PEP8.
+description: ''
+
+# Default Values: https://github.com/sweepai/sweep/blob/main/sweep.yaml


Test patch:
diff --git a/aeon/datasets/tests/test_data_writers.py b/aeon/datasets/tests/test_data_writers.py
index 4c06edf7..97818b5d 100644
--- a/aeon/datasets/tests/test_data_writers.py
+++ b/aeon/datasets/tests/test_data_writers.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
-import shutil
+import os
+import tempfile
 
 import numpy as np
 import pandas as pd
@@ -25,20 +26,20 @@ def test_write_to_tsfile_equal_length(regression, problem_name):
     then deletes the files.
     """
     X, y = make_3d_test_data(regression_target=regression)
-    write_to_tsfile(
-        X=X, path="./Temp/", y=y, problem_name=problem_name, regression=regression
-    )
-    load_path = "./Temp/" + problem_name
-    newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
-    assert isinstance(newX, np.ndarray)
-    assert X.shape == newX.shape
-    assert X[0][0][0] == newX[0][0][0]
-    if not regression:
-        y = y.astype(str)
-        np.testing.assert_array_equal(y, newy)
-    else:
-        np.testing.assert_array_almost_equal(y, newy)
-    shutil.rmtree("./Temp")
+    with tempfile.TemporaryDirectory() as tmp:
+        write_to_tsfile(
+            X=X, path=tmp, y=y, problem_name=problem_name, regression=regression
+        )
+        load_path = os.path.join(tmp, problem_name)
+        newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
+        assert isinstance(newX, np.ndarray)
+        assert X.shape == newX.shape
+        assert X[0][0][0] == newX[0][0][0]
+        if not regression:
+            y = y.astype(str)
+            np.testing.assert_array_equal(y, newy)
+        else:
+            np.testing.assert_array_almost_equal(y, newy)
 
 
 @pytest.mark.parametrize("problem_name", ["Testy", "Testy2.ts"])
@@ -49,15 +50,15 @@ def test_write_regression_to_tsfile_equal_length(problem_name):
     writes locally, reloads, then compares all class labels. It then delete the files.
     """
     X, y = make_3d_test_data(regression_target=True)
-    write_to_tsfile(X=X, path="./Temp/", y=y, problem_name=problem_name)
-    load_path = "./Temp/" + problem_name
-    newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
-    assert isinstance(newX, np.ndarray)
-    assert X.shape == newX.shape
-    assert X[0][0][0] == newX[0][0][0]
-    y = y.astype(str)
-    assert np.array_equal(y, newy)
-    shutil.rmtree("./Temp")
+    with tempfile.TemporaryDirectory() as tmp:
+        write_to_tsfile(X=X, path=tmp, y=y, problem_name=problem_name)
+        load_path = os.path.join(tmp, problem_name)
+        newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
+        assert isinstance(newX, np.ndarray)
+        assert X.shape == newX.shape
+        assert X[0][0][0] == newX[0][0][0]
+        y = y.astype(str)
+        assert np.array_equal(y, newy)
 
 
 @pytest.mark.parametrize("problem_name", ["Testy", "Testy2.ts"])
@@ -68,33 +69,35 @@ def test_write_to_tsfile_unequal_length(problem_name):
     writes locally, reloads, then compares all class labels. It then delete the files.
     """
     X, y = make_unequal_length_test_data()
-    write_to_tsfile(X=X, path="./Temp/", y=y, problem_name=problem_name)
-    load_path = "./Temp/" + problem_name
-    newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
-    assert isinstance(newX, list)
-    assert len(X) == len(newX)
-    assert X[0][0][0] == newX[0][0][0]
-    y = y.astype(str)
-    assert np.array_equal(y, newy)
-    shutil.rmtree("./Temp")
+    with tempfile.TemporaryDirectory() as tmp:
+        write_to_tsfile(X=X, path=tmp, y=y, problem_name=problem_name)
+        load_path = os.path.join(tmp, problem_name)
+        newX, newy = load_from_tsfile(full_file_path_and_name=load_path)
+        assert isinstance(newX, list)
+        assert len(X) == len(newX)
+        assert X[0][0][0] == newX[0][0][0]
+        y = y.astype(str)
+        assert np.array_equal(y, newy)
 
 
-def test_write_dataframe_to_ts():
+@pytest.mark.parametrize("tsfile_writer", [_write_dataframe_to_tsfile, write_to_tsfile])
+def test_write_dataframe_to_ts(tsfile_writer):
     """Tests whether a dataset can be written by the .ts writer then read in."""
     # load an example dataset
     problem_name = "Testy.ts"
     X, y = make_nested_dataframe_data()
-    # output the dataframe in a ts file
-    _write_dataframe_to_tsfile(
-        X=X,
-        path="./Temp/",
-        y=y,
-        problem_name=problem_name,
-    )
-    # load data back from the ts file into dataframe
-    newX, newy = load_from_tsfile_to_dataframe("./Temp/" + problem_name)
-    # check if the dataframes are the same
-    pd.testing.assert_frame_equal(newX, X)
-    y2 = pd.Series(y)
-    pd.testing.assert_series_equal(y, y2)
-    shutil.rmtree("./Temp/")
+    with tempfile.TemporaryDirectory() as tmp:
+        # output the dataframe in a ts file
+        tsfile_writer(
+            X=X,
+            path=tmp,
+            y=y,
+            problem_name=problem_name,
+        )
+        # load data back from the ts file into dataframe
+        load_path = os.path.join(tmp, problem_name)
+        newX, newy = load_from_tsfile_to_dataframe(load_path)
+        # check if the dataframes are the same
+        pd.testing.assert_frame_equal(newX, X)
+        y2 = pd.Series(y)
+        pd.testing.assert_series_equal(y, y2)