From 1654ebbdbe9531a87339d9f175ae33dd2446edc4 Mon Sep 17 00:00:00 2001 From: Matthew Middlehurst Date: Wed, 8 May 2024 15:19:04 +0100 Subject: [PATCH 1/4] start --- aep/05_testing_rework.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 aep/05_testing_rework.md diff --git a/aep/05_testing_rework.md b/aep/05_testing_rework.md new file mode 100644 index 0000000..5db6d0d --- /dev/null +++ b/aep/05_testing_rework.md @@ -0,0 +1,37 @@ +# AEP Template + +Contributors: [list of contributor GitHub handles] + +## Overview + +[Short description of the AEP contents and purpose] + +## Problem Statement and Use Cases + +[Describe the problem that the AEP is trying to solve and the use cases that it is +trying to address] + +[Include an example of how the AEP contents would be used] + +## Implementation + +[Describe the steps required for the implementation of the AEP contents] + +## Examples code/structure (if applicable) + +[Include any example code or structure that would be used to implement the AEP. Links +to a pull request are also acceptable] + +## Considerations and Alternatives + +[Describe any considerations i.e. backwards compatability and alternatives that +were considered] + +## Discussion + +[Include or link to any discussion points that were raised during the AEP process] + +## References + +[Include any references to external resources or publications that are pointed to +in the AEP] \ No newline at end of file From e8b1cb815071fe8a9af0bfae12d724e4f7cfb5e6 Mon Sep 17 00:00:00 2001 From: Matthew Middlehurst Date: Wed, 22 May 2024 19:25:23 +0100 Subject: [PATCH 2/4] content --- aep/05_testing_rework.md | 57 +++++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/aep/05_testing_rework.md b/aep/05_testing_rework.md index 5db6d0d..77f0100 100644 --- a/aep/05_testing_rework.md +++ b/aep/05_testing_rework.md @@ -1,37 +1,68 @@ # AEP Template -Contributors: [list of contributor GitHub handles] +Contributors: MatthewMiddlehurst ## Overview -[Short description of the AEP contents and purpose] +The current general testing framework is confusing and complicated. This AEP aims to +suggest a simplification for the general testing of estimators, based around the +`scikit-learn1` `check_estimator` method of testing. ## Problem Statement and Use Cases -[Describe the problem that the AEP is trying to solve and the use cases that it is -trying to address] +Currently `aeon` uses a number of custom `pytest` fixtures and "scenario" classes to +test estimators. This is generally confusing, both to maintain and to understand the +exact contents of a test being run. This AEP suggests a simplification of the testing +framework to use the `check_estimator` [1] and `parametrize_with_checks` [2] functions +as a basis for general unit testing, similar to what is done in `scikit-learn`. This +would include the removal of the `pytest` fixtures and scenario classes, and a rework +of the current `check_estimator` method which is heavily reliant on the `pytest` +fixtures. -[Include an example of how the AEP contents would be used] +This involves reworking the current tests to be run through the mentioned functions, +rather than being individual pytest `functions`. All tests would be run through a single +function, i.e. + +```python +@parametrize_with_checks(ALL_ESTIMATORS) +def test_parametrize_with_checks_classes(estimator, check): + """Test all checks on all aeon estimators.""" + check(estimator) +``` + +Which would run every test function on every estimator. ## Implementation -[Describe the steps required for the implementation of the AEP contents] +1. Create proof of concept for the new testing framework and functions + ([#1479](https://github.com/aeon-toolkit/aeon/pull/1479)). +2. Port over all existing tests to the new framework, i.e. the current classification + test fixtue and the various `test_all_x` files. +3. Remove all old testing fixtures and scenario classes, swapping them for the new + framework or isolate to legacy frameworks. +4. Consider changing the tests available or adding and removing from the suite + to better meet the new requirements of the package. +5. Update the documentation to reflect the new testing framework. ## Examples code/structure (if applicable) -[Include any example code or structure that would be used to implement the AEP. Links -to a pull request are also acceptable] +See [#1479](https://github.com/aeon-toolkit/aeon/pull/1479) ## Considerations and Alternatives -[Describe any considerations i.e. backwards compatability and alternatives that -were considered] +The testing module is currently marked as experimental, so while there may be some +differences compared to previous testing for users if they use this functionality, +no deprecation is required according to the current document. + +I am currently not including `BaseTransformer` and `BaseForecaster` in this update, +as they are likely to be removed or heavily overhauled in the future. This would require +keeping the old testing framework and isolating the tests for these frameworks. ## Discussion -[Include or link to any discussion points that were raised during the AEP process] +N/A ## References -[Include any references to external resources or publications that are pointed to -in the AEP] \ No newline at end of file +- [1] https://scikit-learn.org/stable/modules/generated/sklearn.utils.estimator_checks.check_estimator.html +- [2] https://scikit-learn.org/stable/modules/generated/sklearn.utils.estimator_checks.parametrize_with_checks.html From 64e312358c2f2a5b20247aade8ad349414221f5f Mon Sep 17 00:00:00 2001 From: Matthew Middlehurst Date: Wed, 22 May 2024 19:27:22 +0100 Subject: [PATCH 3/4] reasoning --- aep/05_testing_rework.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/aep/05_testing_rework.md b/aep/05_testing_rework.md index 77f0100..921243a 100644 --- a/aep/05_testing_rework.md +++ b/aep/05_testing_rework.md @@ -32,6 +32,11 @@ def test_parametrize_with_checks_classes(estimator, check): Which would run every test function on every estimator. +This change will allow us to more easily edit the contents of these tests if necessary +and integrate our new experimental modules into the testing framework. The testing +contents will also be more centralised, rather than split into double digit files +throughout all modules of the package. + ## Implementation 1. Create proof of concept for the new testing framework and functions From 2c37a2cc8dd4b464998ee02cdc58ff3701a1fc6c Mon Sep 17 00:00:00 2001 From: Matthew Middlehurst Date: Tue, 15 Oct 2024 12:10:17 +0100 Subject: [PATCH 4/4] Update 05_testing_rework.md --- aep/05_testing_rework.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/aep/05_testing_rework.md b/aep/05_testing_rework.md index 921243a..abdd3a6 100644 --- a/aep/05_testing_rework.md +++ b/aep/05_testing_rework.md @@ -6,7 +6,7 @@ Contributors: MatthewMiddlehurst The current general testing framework is confusing and complicated. This AEP aims to suggest a simplification for the general testing of estimators, based around the -`scikit-learn1` `check_estimator` method of testing. +`scikit-learn` `check_estimator` method of testing. ## Problem Statement and Use Cases @@ -53,16 +53,19 @@ throughout all modules of the package. See [#1479](https://github.com/aeon-toolkit/aeon/pull/1479) +PRs for part 2 and 3: +- https://github.com/aeon-toolkit/aeon/pull/1770 +- https://github.com/aeon-toolkit/aeon/pull/1875 +- https://github.com/aeon-toolkit/aeon/pull/1877 +- https://github.com/aeon-toolkit/aeon/pull/2076 +- https://github.com/aeon-toolkit/aeon/pull/2086 + ## Considerations and Alternatives The testing module is currently marked as experimental, so while there may be some differences compared to previous testing for users if they use this functionality, no deprecation is required according to the current document. -I am currently not including `BaseTransformer` and `BaseForecaster` in this update, -as they are likely to be removed or heavily overhauled in the future. This would require -keeping the old testing framework and isolating the tests for these frameworks. - ## Discussion N/A