From 7d6cf286f236a5a73cd3e2755b11c3ea4f90d1ff Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Thu, 27 Jun 2019 13:23:39 +0200 Subject: [PATCH 01/17] Collecting and cleaning unit test dump --- openml/testing.py | 19 +++++++++++++++++++ tests/test_datasets/test_dataset_functions.py | 14 +++++++++++--- tests/test_tasks/test_split.py | 6 +++++- tests/test_tasks/test_task_functions.py | 8 ++++++++ tests/test_tasks/test_task_methods.py | 9 +++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 1ce0862d0..614764931 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -103,6 +103,25 @@ def tearDown(self): openml.config.server = self.production_server openml.config.connection_n_retries = self.connection_n_retries + def _track_old_files(self): + files = os.walk(self.static_cache_dir) + self.old_file_list = [] + for root, _, filenames in files: + for filename in filenames: + self.old_file_list.append(os.path.join(root, filename)) + + def _remove_new_files(self): + files = os.walk(self.static_cache_dir) + self.new_file_list = [] + for root, _, filenames in files: + for filename in filenames: + self.new_file_list.append(os.path.join(root, filename)) + # filtering the files generated during this run + self.new_file_list = list(set(self.new_file_list) - + set(self.old_file_list)) + for file in self.new_file_list: + os.remove(file) + def _get_sentinel(self, sentinel=None): if sentinel is None: # Create a unique prefix for the flow. Necessary because the flow diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index 61aeb6904..6d0f5fcf5 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -1,4 +1,5 @@ import os +import sys import random from itertools import product from unittest import mock @@ -6,6 +7,7 @@ import arff import pytest +import shutil import numpy as np import pandas as pd import scipy.sparse @@ -44,18 +46,24 @@ def tearDown(self): def _remove_pickle_files(self): cache_dir = self.static_cache_dir + self.lock_path = os.path.join(openml.config.get_cache_directory(), 'locks') for did in ['-1', '2']: with lockutils.external_lock( name='datasets.functions.get_dataset:%s' % did, - lock_path=os.path.join(openml.config.get_cache_directory(), 'locks'), + lock_path=self.lock_path, ): - pickle_path = os.path.join(cache_dir, 'datasets', did, - 'dataset.pkl') + if sys.version[0] is '3': + pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', + did, 'dataset.pkl.py3') + else: + pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', + did, 'dataset.pkl') try: os.remove(pickle_path) except (OSError, FileNotFoundError): # Replaced a bare except. Not sure why either of these would be acceptable. pass + shutil.rmtree(self.lock_path, ignore_errors=True) def _get_empty_param_for_dataset(self): diff --git a/tests/test_tasks/test_split.py b/tests/test_tasks/test_split.py index 46c6564a1..583667e85 100644 --- a/tests/test_tasks/test_split.py +++ b/tests/test_tasks/test_split.py @@ -1,4 +1,5 @@ import inspect +import sys import os import numpy as np @@ -20,7 +21,10 @@ def setUp(self): "tasks", "1882", "datasplits.arff" ) # TODO Needs to be adapted regarding the python version - self.pd_filename = self.arff_filename.replace(".arff", ".pkl") + if sys.version[0] is '3': + self.pd_filename = self.arff_filename.replace(".arff", ".pkl.py3") + else: + self.pd_filename = self.arff_filename.replace(".arff", ".pkl") def tearDown(self): try: diff --git a/tests/test_tasks/test_task_functions.py b/tests/test_tasks/test_task_functions.py index dfdbd4847..4a6dffd75 100644 --- a/tests/test_tasks/test_task_functions.py +++ b/tests/test_tasks/test_task_functions.py @@ -12,6 +12,14 @@ class TestTask(TestBase): _multiprocess_can_split_ = True + def setUp(self): + super(TestTask, self).setUp() + self._track_old_files() + + def tearDown(self): + self._remove_new_files() + super(TestTask, self).tearDown() + def test__get_cached_tasks(self): openml.config.cache_directory = self.static_cache_dir tasks = openml.tasks.functions._get_cached_tasks() diff --git a/tests/test_tasks/test_task_methods.py b/tests/test_tasks/test_task_methods.py index 55cbba64b..d6d33ce16 100644 --- a/tests/test_tasks/test_task_methods.py +++ b/tests/test_tasks/test_task_methods.py @@ -1,3 +1,4 @@ +import os from time import time import openml @@ -7,6 +8,14 @@ # Common methods between tasks class OpenMLTaskMethodsTest(TestBase): + def setUp(self): + super(OpenMLTaskMethodsTest, self).setUp() + self._track_old_files() + + def tearDown(self): + self._remove_new_files() + super(OpenMLTaskMethodsTest, self).tearDown() + def test_tagging(self): task = openml.tasks.get_task(1) tag = "testing_tag_{}_{}".format(self.id(), time()) From 299477a87edb8e0beb01e1e543fb756e94c82541 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Mon, 1 Jul 2019 16:45:15 +0200 Subject: [PATCH 02/17] Adding session level fixture with yield to delay deletion of files --- openml/testing.py | 55 ++++++++++++------- tests/test_datasets/test_dataset_functions.py | 7 +-- tests/test_tasks/test_split.py | 4 +- tests/test_tasks/test_task_functions.py | 4 +- tests/test_tasks/test_task_methods.py | 5 +- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 614764931..a333282e7 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -17,6 +17,41 @@ import openml from openml.tasks import TaskTypeEnum +import pytest + + +@pytest.fixture(scope="session", autouse=True) +def cleanup_fixture(): + """Cleans up files generated by Unit tests + + This function is called at the beginning of the invocation of + TestBase (defined below), by each of class that inherits TestBase. + The 'yield' creates a checkpoint and breaks away to continue running + the unit tests of the sub class. When all the tests end, execution + resumes from the checkpoint. + """ + # TODO: generalize this path better akin to static_cache_dir + directory = "{}/tests/files/".format(os.getcwd()) + files = os.walk(directory) + old_file_list = [] + for root, _, filenames in files: + for filename in filenames: + old_file_list.append(os.path.join(root, filename)) + # context switches to other remaining tests + # pauses the code execution here till all tests in the 'session' is over + yield + # resumes from here after all collected tests are completed + files = os.walk(directory) + new_file_list = [] + for root, _, filenames in files: + for filename in filenames: + new_file_list.append(os.path.join(root, filename)) + # filtering the files generated during this run + new_file_list = list(set(new_file_list) - set(old_file_list)) + print("Files to delete: {}".format(new_file_list)) + for file in new_file_list: + os.remove(file) + class TestBase(unittest.TestCase): """Base class for tests @@ -44,7 +79,6 @@ def setUp(self, n_levels: int = 1): Number of nested directories the test is in. Necessary to resolve the path to the ``files`` directory, which is located directly under the ``tests`` directory. """ - # This cache directory is checked in to git to simulate a populated # cache self.maxDiff = None @@ -103,25 +137,6 @@ def tearDown(self): openml.config.server = self.production_server openml.config.connection_n_retries = self.connection_n_retries - def _track_old_files(self): - files = os.walk(self.static_cache_dir) - self.old_file_list = [] - for root, _, filenames in files: - for filename in filenames: - self.old_file_list.append(os.path.join(root, filename)) - - def _remove_new_files(self): - files = os.walk(self.static_cache_dir) - self.new_file_list = [] - for root, _, filenames in files: - for filename in filenames: - self.new_file_list.append(os.path.join(root, filename)) - # filtering the files generated during this run - self.new_file_list = list(set(self.new_file_list) - - set(self.old_file_list)) - for file in self.new_file_list: - os.remove(file) - def _get_sentinel(self, sentinel=None): if sentinel is None: # Create a unique prefix for the flow. Necessary because the flow diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index 6d0f5fcf5..a88df795b 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -7,7 +7,6 @@ import arff import pytest -import shutil import numpy as np import pandas as pd import scipy.sparse @@ -17,7 +16,7 @@ from openml import OpenMLDataset from openml.exceptions import OpenMLCacheException, OpenMLHashException, \ OpenMLPrivateDatasetError -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture from openml.utils import _tag_entity, _create_cache_directory_for_id from openml.datasets.functions import (create_dataset, attributes_arff_from_df, @@ -45,14 +44,13 @@ def tearDown(self): super(TestOpenMLDataset, self).tearDown() def _remove_pickle_files(self): - cache_dir = self.static_cache_dir self.lock_path = os.path.join(openml.config.get_cache_directory(), 'locks') for did in ['-1', '2']: with lockutils.external_lock( name='datasets.functions.get_dataset:%s' % did, lock_path=self.lock_path, ): - if sys.version[0] is '3': + if sys.version[0] == '3': pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', did, 'dataset.pkl.py3') else: @@ -63,7 +61,6 @@ def _remove_pickle_files(self): except (OSError, FileNotFoundError): # Replaced a bare except. Not sure why either of these would be acceptable. pass - shutil.rmtree(self.lock_path, ignore_errors=True) def _get_empty_param_for_dataset(self): diff --git a/tests/test_tasks/test_split.py b/tests/test_tasks/test_split.py index 583667e85..92db5505e 100644 --- a/tests/test_tasks/test_split.py +++ b/tests/test_tasks/test_split.py @@ -5,7 +5,7 @@ import numpy as np from openml import OpenMLSplit -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture class OpenMLSplitTest(TestBase): @@ -21,7 +21,7 @@ def setUp(self): "tasks", "1882", "datasplits.arff" ) # TODO Needs to be adapted regarding the python version - if sys.version[0] is '3': + if sys.version[0] == '3': self.pd_filename = self.arff_filename.replace(".arff", ".pkl.py3") else: self.pd_filename = self.arff_filename.replace(".arff", ".pkl") diff --git a/tests/test_tasks/test_task_functions.py b/tests/test_tasks/test_task_functions.py index 4a6dffd75..3d2f08a6a 100644 --- a/tests/test_tasks/test_task_functions.py +++ b/tests/test_tasks/test_task_functions.py @@ -1,7 +1,7 @@ import os from unittest import mock -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture from openml import OpenMLSplit, OpenMLTask from openml.exceptions import OpenMLCacheException import openml @@ -14,10 +14,8 @@ class TestTask(TestBase): def setUp(self): super(TestTask, self).setUp() - self._track_old_files() def tearDown(self): - self._remove_new_files() super(TestTask, self).tearDown() def test__get_cached_tasks(self): diff --git a/tests/test_tasks/test_task_methods.py b/tests/test_tasks/test_task_methods.py index d6d33ce16..0d28d3e8b 100644 --- a/tests/test_tasks/test_task_methods.py +++ b/tests/test_tasks/test_task_methods.py @@ -1,8 +1,7 @@ -import os from time import time import openml -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture # Common methods between tasks @@ -10,10 +9,8 @@ class OpenMLTaskMethodsTest(TestBase): def setUp(self): super(OpenMLTaskMethodsTest, self).setUp() - self._track_old_files() def tearDown(self): - self._remove_new_files() super(OpenMLTaskMethodsTest, self).tearDown() def test_tagging(self): From 5ecafe8b0725b7b51095e264d466e2e804f96bb1 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Wed, 3 Jul 2019 15:20:23 +0200 Subject: [PATCH 03/17] Adding PEP8 ignore F401 --- ci_scripts/flake8_diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci_scripts/flake8_diff.sh b/ci_scripts/flake8_diff.sh index d74577341..696e8c7f3 100755 --- a/ci_scripts/flake8_diff.sh +++ b/ci_scripts/flake8_diff.sh @@ -3,5 +3,5 @@ # Update /CONTRIBUTING.md if these commands change. # The reason for not advocating using this script directly is that it # might not work out of the box on Windows. -flake8 --ignore E402,W503 --show-source --max-line-length 100 $options +flake8 --ignore E402,W503,F401 --show-source --max-line-length 100 $options mypy openml --ignore-missing-imports --follow-imports skip From 758aa3082931144051a234b1f7ffe56f6a38e261 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Wed, 3 Jul 2019 15:29:49 +0200 Subject: [PATCH 04/17] Changelog update + pytest argument fix --- ci_scripts/test.sh | 2 +- doc/progress.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ci_scripts/test.sh b/ci_scripts/test.sh index 80b35f04f..2a837583e 100644 --- a/ci_scripts/test.sh +++ b/ci_scripts/test.sh @@ -22,7 +22,7 @@ run_tests() { PYTEST_ARGS='' fi - pytest -n 4 --duration=20 --timeout=600 --timeout-method=thread -sv --ignore='test_OpenMLDemo.py' $PYTEST_ARGS $test_dir + pytest -n 4 --durations=20 --timeout=600 --timeout-method=thread -sv --ignore='test_OpenMLDemo.py' $PYTEST_ARGS $test_dir } if [[ "$RUN_FLAKE8" == "true" ]]; then diff --git a/doc/progress.rst b/doc/progress.rst index 83e46d494..bc00fd300 100644 --- a/doc/progress.rst +++ b/doc/progress.rst @@ -11,6 +11,7 @@ Changelog * ADD #687: Adds a function to retrieve the list of evaluation measures available. * ADD #695: A function to retrieve all the data quality measures available. +* FIX #447: All files created by unit tests no longer persist. 0.9.0 ~~~~~ From fa776100f0dd98853c67c24a4fae9b7cec0b1e71 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Wed, 3 Jul 2019 17:49:00 +0200 Subject: [PATCH 05/17] Messy first draft of possible designs --- openml/testing.py | 109 +++++++++++++++++++++++ tests/test_flows/test_flow.py | 24 +++-- tests/test_flows/test_flow_functions.py | 4 +- tests/test_study/test_study_functions.py | 5 +- 4 files changed, 135 insertions(+), 7 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 1ce0862d0..f7070a8cf 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -17,6 +17,54 @@ import openml from openml.tasks import TaskTypeEnum +import pytest + + +@pytest.fixture(scope="session") #, autouse=True) +def cleanup_fixture(): + """Cleans up files generated by Unit tests + + This function is called at the beginning of the invocation of + TestBase (defined below), by each of class that inherits TestBase. + The 'yield' creates a checkpoint and breaks away to continue running + the unit tests of the sub class. When all the tests end, execution + resumes from the checkpoint. + """ + # TODO: generalize this path better akin to static_cache_dir + directory = "{}/tests/files/".format(os.getcwd()) + files = os.walk(directory) + old_file_list = [] + for root, _, filenames in files: + for filename in filenames: + old_file_list.append(os.path.join(root, filename)) + # context switches to other remaining tests + # pauses the code execution here till all tests in the 'session' is over + yield + # resumes from here after all collected tests are completed + files = os.walk(directory) + new_file_list = [] + for root, _, filenames in files: + for filename in filenames: + new_file_list.append(os.path.join(root, filename)) + # filtering the files generated during this run + new_file_list = list(set(new_file_list) - set(old_file_list)) + print("Files to delete: {}".format(new_file_list)) + for file in new_file_list: + os.remove(file) + + # Removing files from test server + print("TestBase.tracker: {}".format(TestBase.tracker)) + openml.config.server = TestBase.test_server + openml.config.apikey = TestBase.apikey + for entity_type in TestBase.tracker: + for i, entity in enumerate(TestBase.tracker[entity_type]): + try: + print("Deleting: {} {}". format(entity_type, entity)) + openml.utils._delete_entity(entity_type, entity) + TestBase.tracker[entity_type].pop(i) + except Exception as e: + print(e) + class TestBase(unittest.TestCase): """Base class for tests @@ -26,6 +74,61 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ + tracker = {} + test_server = None + apikey = None + + @classmethod + def _track_test_server_dumps(self, entity_type, entity_id): + if entity_type not in TestBase.tracker: + TestBase.tracker[entity_type] = [entity_id] + else: + TestBase.tracker[entity_type].append(entity_id) + + @pytest.fixture(scope="session", autouse=True) + def _cleanup_fixture(self): + """Cleans up files generated by Unit tests + + This function is called at the beginning of the invocation of + TestBase (defined below), by each of class that inherits TestBase. + The 'yield' creates a checkpoint and breaks away to continue running + the unit tests of the sub class. When all the tests end, execution + resumes from the checkpoint. + """ + # TODO: generalize this path better akin to static_cache_dir + directory = "{}/tests/files/".format(os.getcwd()) + files = os.walk(directory) + old_file_list = [] + for root, _, filenames in files: + for filename in filenames: + old_file_list.append(os.path.join(root, filename)) + # context switches to other remaining tests + # pauses the code execution here till all tests in the 'session' is over + yield + # resumes from here after all collected tests are completed + files = os.walk(directory) + new_file_list = [] + for root, _, filenames in files: + for filename in filenames: + new_file_list.append(os.path.join(root, filename)) + # filtering the files generated during this run + new_file_list = list(set(new_file_list) - set(old_file_list)) + print("Files to delete: {}".format(new_file_list)) + for file in new_file_list: + os.remove(file) + + # Removing files from test server + print("TestBase.tracker: {}".format(TestBase.tracker)) + openml.config.server = TestBase.test_server + openml.config.apikey = TestBase.apikey + for entity_type in TestBase.tracker: + for i, entity in enumerate(TestBase.tracker[entity_type]): + try: + print("Deleting: {} {}". format(entity_type, entity)) + openml.utils._delete_entity(entity_type, entity) + TestBase.tracker[entity_type].pop(i) + except Exception as e: + print(e) def setUp(self, n_levels: int = 1): """Setup variables and temporary directories. @@ -75,6 +178,10 @@ def setUp(self, n_levels: int = 1): self.production_server = "https://openml.org/api/v1/xml" self.test_server = "https://test.openml.org/api/v1/xml" + # For global file deletion on test server + TestBase.test_server = self.test_server + TestBase.apikey = openml.config.apikey + openml.config.server = self.test_server openml.config.avoid_duplicate_runs = False openml.config.cache_directory = self.workdir @@ -91,6 +198,8 @@ def setUp(self, n_levels: int = 1): openml.config.connection_n_retries = 10 def tearDown(self): + print("\nTRACKER: {}".format(TestBase.tracker)) + os.chdir(self.cwd) try: shutil.rmtree(self.workdir) diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index 7b8c66cab..8b72be909 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -30,7 +30,7 @@ from openml._api_calls import _perform_api_call import openml.exceptions import openml.extensions.sklearn -from openml.testing import TestBase +from openml.testing import TestBase #, cleanup_fixture import openml.utils @@ -41,6 +41,10 @@ def setUp(self): super().setUp() self.extension = openml.extensions.sklearn.SklearnExtension() + def tearDown(self): + # TestBase.tracker.append(TestFlow.tracker) + super().tearDown() + def test_get_flow(self): # We need to use the production server here because 4024 is not the # test server @@ -176,7 +180,9 @@ def test_publish_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) - flow.publish() + temp = flow.publish() + self._track_test_server_dumps('flow', flow.flow_id) + print("\ntest_flow: {}".format(flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @mock.patch('openml.flows.functions.flow_exists') @@ -186,7 +192,10 @@ def test_publish_existing_flow(self, flow_exists_mock): flow_exists_mock.return_value = 1 with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: - flow.publish(raise_error_if_exists=True) + temp = flow.publish(raise_error_if_exists=True) + self._track_test_server_dumps('flow', flow.flow_id) + print("\ntest_flow: {}".format(flow.flow_id)) + print(TestFlow.tracker) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -196,7 +205,9 @@ def test_publish_flow_with_similar_components(self): ]) flow = self.extension.model_to_flow(clf) flow, _ = self._add_sentinel_to_flow_name(flow, None) - flow.publish() + temp = flow.publish() + self._track_test_server_dumps('flow', flow.flow_id) + print("\ntest_flow: {}".format(flow.flow_id)) # For a flow where both components are published together, the upload # date should be equal self.assertEqual( @@ -212,7 +223,10 @@ def test_publish_flow_with_similar_components(self): clf1 = sklearn.tree.DecisionTreeClassifier(max_depth=2) flow1 = self.extension.model_to_flow(clf1) flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) - flow1.publish() + temp = flow1.publish() + self._track_test_server_dumps('flow', flow1.flow_id) + print("\ntest_flow: {}".format(flow.flow_id)) + print(TestFlow.tracker) # In order to assign different upload times to the flows! time.sleep(1) diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index 087623d3d..32c8e1efb 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -7,7 +7,7 @@ import pandas as pd import openml -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture import openml.extensions.sklearn @@ -251,6 +251,8 @@ def test_sklearn_to_flow_list_of_lists(self): # Test flow is accepted by server self._add_sentinel_to_flow_name(flow) flow.publish() + TestBase._track_test_server_dumps('flow', flow.flow_id) + print("\ntest_flow_functions: {}".format(flow.flow_id)) # Test deserialization works server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index c87dd8e15..33725f668 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -1,6 +1,6 @@ import openml import openml.study -from openml.testing import TestBase +from openml.testing import TestBase, cleanup_fixture import pandas as pd @@ -77,6 +77,9 @@ def test_publish_benchmark_suite(self): task_ids=fixture_task_ids ) study_id = study.publish() + self._track_test_server_dumps('study', study_id) + print("\ntest_study_functions: {}".format(study_id)) + self.assertGreater(study_id, 0) # verify main meta data From 2bc519e3a652d985a82d452186be48d5dfebcea8 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Thu, 4 Jul 2019 16:37:50 +0200 Subject: [PATCH 06/17] Leaner implementation without additional imports --- ci_scripts/flake8_diff.sh | 2 +- openml/testing.py | 65 +++++++++---------- tests/test_datasets/test_dataset_functions.py | 2 +- tests/test_tasks/test_split.py | 2 +- tests/test_tasks/test_task_functions.py | 2 +- tests/test_tasks/test_task_methods.py | 2 +- 6 files changed, 37 insertions(+), 38 deletions(-) diff --git a/ci_scripts/flake8_diff.sh b/ci_scripts/flake8_diff.sh index 696e8c7f3..d74577341 100755 --- a/ci_scripts/flake8_diff.sh +++ b/ci_scripts/flake8_diff.sh @@ -3,5 +3,5 @@ # Update /CONTRIBUTING.md if these commands change. # The reason for not advocating using this script directly is that it # might not work out of the box on Windows. -flake8 --ignore E402,W503,F401 --show-source --max-line-length 100 $options +flake8 --ignore E402,W503 --show-source --max-line-length 100 $options mypy openml --ignore-missing-imports --follow-imports skip diff --git a/openml/testing.py b/openml/testing.py index a333282e7..0162379f1 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -20,39 +20,6 @@ import pytest -@pytest.fixture(scope="session", autouse=True) -def cleanup_fixture(): - """Cleans up files generated by Unit tests - - This function is called at the beginning of the invocation of - TestBase (defined below), by each of class that inherits TestBase. - The 'yield' creates a checkpoint and breaks away to continue running - the unit tests of the sub class. When all the tests end, execution - resumes from the checkpoint. - """ - # TODO: generalize this path better akin to static_cache_dir - directory = "{}/tests/files/".format(os.getcwd()) - files = os.walk(directory) - old_file_list = [] - for root, _, filenames in files: - for filename in filenames: - old_file_list.append(os.path.join(root, filename)) - # context switches to other remaining tests - # pauses the code execution here till all tests in the 'session' is over - yield - # resumes from here after all collected tests are completed - files = os.walk(directory) - new_file_list = [] - for root, _, filenames in files: - for filename in filenames: - new_file_list.append(os.path.join(root, filename)) - # filtering the files generated during this run - new_file_list = list(set(new_file_list) - set(old_file_list)) - print("Files to delete: {}".format(new_file_list)) - for file in new_file_list: - os.remove(file) - - class TestBase(unittest.TestCase): """Base class for tests @@ -62,6 +29,38 @@ class TestBase(unittest.TestCase): Hopefully soon allows using a test server, not the production server. """ + @pytest.fixture(scope="session", autouse=True) + def _cleanup_fixture(self): + """Cleans up files generated by Unit tests + + This function is called at the beginning of the invocation of + TestBase (defined below), by each of class that inherits TestBase. + The 'yield' creates a checkpoint and breaks away to continue running + the unit tests of the sub class. When all the tests end, execution + resumes from the checkpoint. + """ + # TODO: generalize this path better akin to static_cache_dir + directory = "{}/tests/files/".format(os.getcwd()) + files = os.walk(directory) + old_file_list = [] + for root, _, filenames in files: + for filename in filenames: + old_file_list.append(os.path.join(root, filename)) + # context switches to other remaining tests + # pauses the code execution here till all tests in the 'session' is over + yield + # resumes from here after all collected tests are completed + files = os.walk(directory) + new_file_list = [] + for root, _, filenames in files: + for filename in filenames: + new_file_list.append(os.path.join(root, filename)) + # filtering the files generated during this run + new_file_list = list(set(new_file_list) - set(old_file_list)) + print("Files to delete in local: {}".format(new_file_list)) + for file in new_file_list: + os.remove(file) + def setUp(self, n_levels: int = 1): """Setup variables and temporary directories. diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index a88df795b..fb2270952 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -16,7 +16,7 @@ from openml import OpenMLDataset from openml.exceptions import OpenMLCacheException, OpenMLHashException, \ OpenMLPrivateDatasetError -from openml.testing import TestBase, cleanup_fixture +from openml.testing import TestBase from openml.utils import _tag_entity, _create_cache_directory_for_id from openml.datasets.functions import (create_dataset, attributes_arff_from_df, diff --git a/tests/test_tasks/test_split.py b/tests/test_tasks/test_split.py index 92db5505e..bbb2a2600 100644 --- a/tests/test_tasks/test_split.py +++ b/tests/test_tasks/test_split.py @@ -5,7 +5,7 @@ import numpy as np from openml import OpenMLSplit -from openml.testing import TestBase, cleanup_fixture +from openml.testing import TestBase class OpenMLSplitTest(TestBase): diff --git a/tests/test_tasks/test_task_functions.py b/tests/test_tasks/test_task_functions.py index 3d2f08a6a..f773752d5 100644 --- a/tests/test_tasks/test_task_functions.py +++ b/tests/test_tasks/test_task_functions.py @@ -1,7 +1,7 @@ import os from unittest import mock -from openml.testing import TestBase, cleanup_fixture +from openml.testing import TestBase from openml import OpenMLSplit, OpenMLTask from openml.exceptions import OpenMLCacheException import openml diff --git a/tests/test_tasks/test_task_methods.py b/tests/test_tasks/test_task_methods.py index 0d28d3e8b..4a0789414 100644 --- a/tests/test_tasks/test_task_methods.py +++ b/tests/test_tasks/test_task_methods.py @@ -1,7 +1,7 @@ from time import time import openml -from openml.testing import TestBase, cleanup_fixture +from openml.testing import TestBase # Common methods between tasks From 10c8dd8477fdea63c37085c5e767239b8f3510c7 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Tue, 9 Jul 2019 20:17:47 +0200 Subject: [PATCH 07/17] Reordering flows to delete subflows later --- ci_scripts/test.sh | 12 +++++++ doc/progress.rst | 4 ++- openml/evaluations/functions.py | 14 +++++++- openml/flows/flow.py | 10 +++++- openml/flows/functions.py | 3 +- openml/runs/functions.py | 9 ++++++ openml/testing.py | 26 +++++++++++++-- openml/utils.py | 3 +- tests/test_datasets/test_dataset_functions.py | 9 ++---- .../test_evaluation_functions.py | 21 ++++++++++++ tests/test_flows/test_flow.py | 5 ++- tests/test_flows/test_flow_functions.py | 32 +++++++++++++++++++ tests/test_runs/test_run_functions.py | 4 +-- tests/test_tasks/test_clustering_task.py | 1 + tests/test_tasks/test_split.py | 7 +--- 15 files changed, 133 insertions(+), 27 deletions(-) diff --git a/ci_scripts/test.sh b/ci_scripts/test.sh index 2a837583e..51ecace49 100644 --- a/ci_scripts/test.sh +++ b/ci_scripts/test.sh @@ -1,5 +1,9 @@ set -e +# check status and branch before running the unit tests +before="`git status --porcelain -b`" +before="$before" + run_tests() { # Get into a temp directory to run test from the installed scikit learn and # check if we do not leave artifacts @@ -32,3 +36,11 @@ fi if [[ "$SKIP_TESTS" != "true" ]]; then run_tests fi + +# check status and branch after running the unit tests +# compares with $before to check for remaining files +after="`git status --porcelain -b`" +if [[ "$before" != "$after" ]]; then + echo "All generated files have not been deleted!" + exit 1 +fi \ No newline at end of file diff --git a/doc/progress.rst b/doc/progress.rst index 739c17f79..70144f64c 100644 --- a/doc/progress.rst +++ b/doc/progress.rst @@ -11,10 +11,12 @@ Changelog * FIX #261: Test server is cleared of all files uploaded during unit testing. * FIX #447: All files created by unit tests no longer persist in local. * FIX #589: Fixing a bug that did not successfully upload the columns to ignore when creating and publishing a dataset. +* FIX #608: Fixing dataset_id referenced before assignment error in get_run function. * DOC #639: More descriptive documention for function to convert array format. * ADD #687: Adds a function to retrieve the list of evaluation measures available. * ADD #695: A function to retrieve all the data quality measures available. - +* ADD #715: `list_evaluations` now has an option to sort evaluations by score (value). +* ADD #722: Automatic reinstantiation of flow in `run_model_on_task`. Clearer errors if that's not possible. 0.9.0 ~~~~~ diff --git a/openml/evaluations/functions.py b/openml/evaluations/functions.py index 72dd6ba4e..37789a752 100644 --- a/openml/evaluations/functions.py +++ b/openml/evaluations/functions.py @@ -2,6 +2,7 @@ import xmltodict import pandas as pd from typing import Union, List, Optional, Dict +import collections import openml.utils import openml._api_calls @@ -19,6 +20,7 @@ def list_evaluations( uploader: Optional[List] = None, tag: Optional[str] = None, per_fold: Optional[bool] = None, + sort_order: Optional[str] = None, output_format: str = 'object' ) -> Union[Dict, pd.DataFrame]: """ @@ -48,6 +50,9 @@ def list_evaluations( per_fold : bool, optional + sort_order : str, optional + order of sorting evaluations, ascending ("asc") or descending ("desc") + output_format: str, optional (default='object') The parameter decides the format of the output. - If 'object' the output is a dict of OpenMLEvaluation objects @@ -77,6 +82,7 @@ def list_evaluations( flow=flow, uploader=uploader, tag=tag, + sort_order=sort_order, per_fold=per_fold_str) @@ -87,6 +93,7 @@ def _list_evaluations( setup: Optional[List] = None, flow: Optional[List] = None, uploader: Optional[List] = None, + sort_order: Optional[str] = None, output_format: str = 'object', **kwargs ) -> Union[Dict, pd.DataFrame]: @@ -114,6 +121,9 @@ def _list_evaluations( kwargs: dict, optional Legal filter operators: tag, limit, offset. + sort_order : str, optional + order of sorting evaluations, ascending ("asc") or descending ("desc") + output_format: str, optional (default='dict') The parameter decides the format of the output. - If 'dict' the output is a dict of dict @@ -141,6 +151,8 @@ def _list_evaluations( api_call += "/flow/%s" % ','.join([str(int(i)) for i in flow]) if uploader is not None: api_call += "/uploader/%s" % ','.join([str(int(i)) for i in uploader]) + if sort_order is not None: + api_call += "/sort_order/%s" % sort_order return __list_evaluations(api_call, output_format=output_format) @@ -157,7 +169,7 @@ def __list_evaluations(api_call, output_format='object'): assert type(evals_dict['oml:evaluations']['oml:evaluation']) == list, \ type(evals_dict['oml:evaluations']) - evals = dict() + evals = collections.OrderedDict() for eval_ in evals_dict['oml:evaluations']['oml:evaluation']: run_id = int(eval_['oml:run_id']) value = None diff --git a/openml/flows/flow.py b/openml/flows/flow.py index c064cef33..bdd4fe6a6 100644 --- a/openml/flows/flow.py +++ b/openml/flows/flow.py @@ -132,7 +132,15 @@ def __init__(self, name, description, model, components, parameters, self.dependencies = dependencies self.flow_id = flow_id - self.extension = get_extension_by_flow(self) + self._extension = get_extension_by_flow(self) + + @property + def extension(self): + if self._extension is not None: + return self._extension + else: + raise RuntimeError("No extension could be found for flow {}: {}" + .format(self.flow_id, self.name)) def __str__(self): header = "OpenML Flow" diff --git a/openml/flows/functions.py b/openml/flows/functions.py index 5841dc699..53a1fdc0a 100644 --- a/openml/flows/functions.py +++ b/openml/flows/functions.py @@ -92,7 +92,6 @@ def get_flow(flow_id: int, reinstantiate: bool = False) -> OpenMLFlow: if reinstantiate: flow.model = flow.extension.flow_to_model(flow) - return flow @@ -360,7 +359,7 @@ def assert_flows_equal(flow1: OpenMLFlow, flow2: OpenMLFlow, assert_flows_equal(attr1[name], attr2[name], ignore_parameter_values_on_older_children, ignore_parameter_values) - elif key == 'extension': + elif key == '_extension': continue else: if key == 'parameters': diff --git a/openml/runs/functions.py b/openml/runs/functions.py index abad7fff8..767a4a48a 100644 --- a/openml/runs/functions.py +++ b/openml/runs/functions.py @@ -171,6 +171,8 @@ def run_flow_on_task( if task.task_id is None: raise ValueError("The task should be published at OpenML") + if flow.model is None: + flow.model = flow.extension.flow_to_model(flow) flow.model = flow.extension.seed_model(flow.model, seed=seed) # We only need to sync with the server right now if we want to upload the flow, @@ -667,6 +669,13 @@ def obtain_field(xml_obj, fieldname, from_server, cast=None): dataset_id = int(run['oml:input_data']['oml:dataset']['oml:did']) elif not from_server: dataset_id = None + else: + # fetching the task to obtain dataset_id + t = openml.tasks.get_task(task_id, download_data=False) + if not hasattr(t, 'dataset_id'): + raise ValueError("Unable to fetch dataset_id from the task({}) " + "linked to run({})".format(task_id, run_id)) + dataset_id = t.dataset_id files = OrderedDict() evaluations = OrderedDict() diff --git a/openml/testing.py b/openml/testing.py index bfead021f..444e7d9ae 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -129,8 +129,17 @@ def _cleanup_fixture(self): the unit tests of the sub class. When all the tests end, execution resumes from the checkpoint. """ - # TODO: generalize this path better akin to static_cache_dir - directory = "{}/tests/files/".format(os.getcwd()) + + abspath_this_file = os.path.abspath(inspect.getfile(self.__class__)) + static_cache_dir = os.path.dirname(abspath_this_file) + # Could be a risky while condition, however, going up a directory + # n-times will eventually end at main directory + while True: + if 'openml' in os.listdir(static_cache_dir): + break + else: + static_cache_dir = os.path.join(static_cache_dir, '../') + directory = os.path.join(static_cache_dir, 'tests/files/') files = os.walk(directory) old_file_list = [] for root, _, filenames in files: @@ -147,7 +156,7 @@ def _cleanup_fixture(self): new_file_list.append(os.path.join(root, filename)) # filtering the files generated during this run new_file_list = list(set(new_file_list) - set(old_file_list)) - print("Deleting local files generated...") + print("Files to delete in local: {}".format(new_file_list)) for file in new_file_list: os.remove(file) @@ -163,6 +172,17 @@ def _cleanup_fixture(self): # putting 'run' in the start of the list entity_types[0], entity_types[index] = entity_types[index], entity_types[0] + # reordering to delete sub flows later + if 'flow' in entity_types: + flows = {} + for entity in TestBase.tracker['flow']: + flows[openml.flows.get_flow(entity).name] = entity + # reordering flow names in descending order of their flow name lengths + flow_deletion_order = [flows[name] for name in sorted(list(flows.keys()), + key=lambda x:len(x), + reverse=True)] + TestBase.tracker['flow'] = flow_deletion_order + for entity_type in entity_types: for i, entity in enumerate(TestBase.tracker[entity_type]): try: diff --git a/openml/utils.py b/openml/utils.py index 54064aca5..f6cc81ff7 100644 --- a/openml/utils.py +++ b/openml/utils.py @@ -5,6 +5,7 @@ import warnings import pandas as pd from functools import wraps +import collections import openml._api_calls import openml.exceptions @@ -182,7 +183,7 @@ def _list_all(listing_call, output_format='dict', *args, **filters): active_filters = {key: value for key, value in filters.items() if value is not None} page = 0 - result = {} + result = collections.OrderedDict() if output_format == 'dataframe': result = pd.DataFrame() diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index 1d99fc3ee..d2c8e091d 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -1,5 +1,4 @@ import os -import sys import random from itertools import product from unittest import mock @@ -50,12 +49,8 @@ def _remove_pickle_files(self): name='datasets.functions.get_dataset:%s' % did, lock_path=self.lock_path, ): - if sys.version[0] == '3': - pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', - did, 'dataset.pkl.py3') - else: - pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', - did, 'dataset.pkl') + pickle_path = os.path.join(openml.config.get_cache_directory(), 'datasets', + did, 'dataset.pkl.py3') try: os.remove(pickle_path) except (OSError, FileNotFoundError): diff --git a/tests/test_evaluations/test_evaluation_functions.py b/tests/test_evaluations/test_evaluation_functions.py index 511f2504b..fecf4b60c 100644 --- a/tests/test_evaluations/test_evaluation_functions.py +++ b/tests/test_evaluations/test_evaluation_functions.py @@ -117,6 +117,27 @@ def test_evaluation_list_per_fold(self): self.assertIsNotNone(evaluations[run_id].value) self.assertIsNone(evaluations[run_id].values) + def test_evaluation_list_sort(self): + size = 10 + task_id = 115 + # Get all evaluations of the task + unsorted_eval = openml.evaluations.list_evaluations( + "predictive_accuracy", offset=0, task=[task_id]) + # Get top 10 evaluations of the same task + sorted_eval = openml.evaluations.list_evaluations( + "predictive_accuracy", size=size, offset=0, task=[task_id], sort_order="desc") + self.assertEqual(len(sorted_eval), size) + self.assertGreater(len(unsorted_eval), 0) + sorted_output = [evaluation.value for evaluation in sorted_eval.values()] + unsorted_output = [evaluation.value for evaluation in unsorted_eval.values()] + + # Check if output from sort is sorted in the right order + self.assertTrue(sorted(sorted_output, reverse=True) == sorted_output) + + # Compare manual sorting against sorted output + test_output = sorted(unsorted_output, reverse=True) + self.assertTrue(test_output[:size] == sorted_output) + def test_list_evaluation_measures(self): measures = openml.evaluations.list_evaluation_measures() self.assertEqual(isinstance(measures, list), True) diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index 65bbe1227..e8dbecb9e 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -42,7 +42,6 @@ def setUp(self): self.extension = openml.extensions.sklearn.SklearnExtension() def tearDown(self): - # TestBase.tracker.append(TestFlow.tracker) super().tearDown() def test_get_flow(self): @@ -280,8 +279,8 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): get_flow_mock.return_value = flow flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + # Not collecting flow_id for deletion since this is a test for failed upload + self.assertEqual(api_call_mock.call_count, 1) self.assertEqual(get_flow_mock.call_count, 1) self.assertEqual(flow_exists_mock.call_count, 1) diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index f4e581a46..f504105dd 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -14,6 +14,12 @@ class TestFlowFunctions(TestBase): _multiprocess_can_split_ = True + def setUp(self): + super().setUp() + + def tearDown(self): + super().tearDown() + def _check_flow(self, flow): self.assertEqual(type(flow), dict) self.assertEqual(len(flow), 6) @@ -258,3 +264,29 @@ def test_sklearn_to_flow_list_of_lists(self): server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertEqual(server_flow.parameters['categories'], '[[0, 1], [0, 1]]') self.assertEqual(server_flow.model.categories, flow.model.categories) + + def test_get_flow_reinstantiate_model(self): + model = sklearn.ensemble.RandomForestClassifier(n_estimators=33) + extension = openml.extensions.get_extension_by_model(model) + flow = extension.model_to_flow(model) + flow.publish(raise_error_if_exists=False) + TestBase._track_test_server_dumps('flow', flow.flow_id) + print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + + downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) + self.assertIsInstance(downloaded_flow.model, sklearn.ensemble.RandomForestClassifier) + + def test_get_flow_reinstantiate_model_no_extension(self): + # Flow 10 is a WEKA flow + self.assertRaisesRegex(RuntimeError, + "No extension could be found for flow 10: weka.SMO", + openml.flows.get_flow, + flow_id=10, + reinstantiate=True) + + @unittest.skipIf(LooseVersion(sklearn.__version__) == "0.20.0", + reason="No non-0.20 scikit-learn flow known.") + def test_get_flow_reinstantiate_model_wrong_version(self): + # 20 is scikit-learn ==0.20.0 + # I can't find a != 0.20 permanent flow on the test server. + self.assertRaises(ValueError, openml.flows.get_flow, flow_id=20, reinstantiate=True) diff --git a/tests/test_runs/test_run_functions.py b/tests/test_runs/test_run_functions.py index e4fc227e4..337a05df6 100644 --- a/tests/test_runs/test_run_functions.py +++ b/tests/test_runs/test_run_functions.py @@ -1277,14 +1277,14 @@ def test_get_uncached_run(self): with self.assertRaises(openml.exceptions.OpenMLCacheException): openml.runs.functions._get_cached_run(10) - def test_run_model_on_task_downloaded_flow(self): + def test_run_flow_on_task_downloaded_flow(self): model = sklearn.ensemble.RandomForestClassifier(n_estimators=33) flow = self.extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) TestBase._track_test_server_dumps('flow', flow.flow_id) print("\ncollected from test_run_functions: {}".format(flow.flow_id)) - downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) + downloaded_flow = openml.flows.get_flow(flow.flow_id) task = openml.tasks.get_task(119) # diabetes run = openml.runs.run_flow_on_task( flow=downloaded_flow, diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index 645a5d3e9..ddaf9bf05 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,4 +1,5 @@ import openml +from openml.testing import import TestBase from .test_task import OpenMLTaskTest diff --git a/tests/test_tasks/test_split.py b/tests/test_tasks/test_split.py index bbb2a2600..763bb15f7 100644 --- a/tests/test_tasks/test_split.py +++ b/tests/test_tasks/test_split.py @@ -1,5 +1,4 @@ import inspect -import sys import os import numpy as np @@ -20,11 +19,7 @@ def setUp(self): self.directory, "..", "files", "org", "openml", "test", "tasks", "1882", "datasplits.arff" ) - # TODO Needs to be adapted regarding the python version - if sys.version[0] == '3': - self.pd_filename = self.arff_filename.replace(".arff", ".pkl.py3") - else: - self.pd_filename = self.arff_filename.replace(".arff", ".pkl") + self.pd_filename = self.arff_filename.replace(".arff", ".pkl.py3") def tearDown(self): try: From fb97ba1bfa7ac08b09c452de1bb2ea44bb5b1839 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Thu, 11 Jul 2019 17:54:03 +0200 Subject: [PATCH 08/17] Updating with design changes for tracking files for deletion --- openml/testing.py | 13 ++++++------- tests/test_flows/test_flow_functions.py | 14 +++++++------- tests/test_tasks/test_clustering_task.py | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 444e7d9ae..8acc8ff38 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -95,7 +95,7 @@ def setUp(self, n_levels: int = 1): with open(openml.config.config_file, 'w') as fh: fh.write('apikey = %s' % openml.config.apikey) - # Increase the number of retries to avoid spurios server failures + # Increase the number of retries to avoid spurious server failures self.connection_n_retries = openml.config.connection_n_retries openml.config.connection_n_retries = 10 @@ -149,6 +149,7 @@ def _cleanup_fixture(self): # pauses the code execution here till all tests in the 'session' is over yield # resumes from here after all collected tests are completed + # local file deletion files = os.walk(directory) new_file_list = [] for root, _, filenames in files: @@ -156,15 +157,12 @@ def _cleanup_fixture(self): new_file_list.append(os.path.join(root, filename)) # filtering the files generated during this run new_file_list = list(set(new_file_list) - set(old_file_list)) - print("Files to delete in local: {}".format(new_file_list)) for file in new_file_list: os.remove(file) # Test server deletion - print("Deleting files uploaded to test server...") openml.config.server = TestBase.test_server openml.config.apikey = TestBase.apikey - entity_types = list(TestBase.tracker.keys()) # deleting 'run' first to allow other dependent entities to be deleted if 'run' in entity_types: @@ -175,14 +173,15 @@ def _cleanup_fixture(self): # reordering to delete sub flows later if 'flow' in entity_types: flows = {} - for entity in TestBase.tracker['flow']: - flows[openml.flows.get_flow(entity).name] = entity + for entity_id, entity_name in TestBase.tracker['flow']: + flows[entity_name] = entity_id # reordering flow names in descending order of their flow name lengths flow_deletion_order = [flows[name] for name in sorted(list(flows.keys()), - key=lambda x:len(x), + key=lambda x: len(x), reverse=True)] TestBase.tracker['flow'] = flow_deletion_order + # deleting all collected entities published to test server for entity_type in entity_types: for i, entity in enumerate(TestBase.tracker[entity_type]): try: diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index f504105dd..a1b45d586 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -4,8 +4,10 @@ from distutils.version import LooseVersion import sklearn +from sklearn import ensemble import pandas as pd +import os import openml from openml.testing import TestBase import openml.extensions.sklearn @@ -15,10 +17,10 @@ class TestFlowFunctions(TestBase): _multiprocess_can_split_ = True def setUp(self): - super().setUp() + super(TestFlowFunctions, self).setUp() def tearDown(self): - super().tearDown() + super(TestFlowFunctions, self).tearDown() def _check_flow(self, flow): self.assertEqual(type(flow), dict) @@ -248,7 +250,6 @@ def test_are_flows_equal_ignore_if_older(self): def test_sklearn_to_flow_list_of_lists(self): from sklearn.preprocessing import OrdinalEncoder ordinal_encoder = OrdinalEncoder(categories=[[0, 1], [0, 1]]) - extension = openml.extensions.sklearn.SklearnExtension() # Test serialization works @@ -257,20 +258,19 @@ def test_sklearn_to_flow_list_of_lists(self): # Test flow is accepted by server self._add_sentinel_to_flow_name(flow) flow.publish() - TestBase._track_test_server_dumps('flow', flow.flow_id) + TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) - # Test deserialization works server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertEqual(server_flow.parameters['categories'], '[[0, 1], [0, 1]]') self.assertEqual(server_flow.model.categories, flow.model.categories) def test_get_flow_reinstantiate_model(self): - model = sklearn.ensemble.RandomForestClassifier(n_estimators=33) + model = ensemble.RandomForestClassifier(n_estimators=33) extension = openml.extensions.get_extension_by_model(model) flow = extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) - TestBase._track_test_server_dumps('flow', flow.flow_id) + TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index ddaf9bf05..7c6e150b2 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,5 +1,5 @@ import openml -from openml.testing import import TestBase +from openml.testing import TestBase from .test_task import OpenMLTaskTest From 6f4fb5f634467ebc158c8a4cfe3fa2caa8f4630c Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Thu, 11 Jul 2019 23:05:16 +0200 Subject: [PATCH 09/17] Handling edge cases --- openml/testing.py | 40 +++++++++++++++++++---- tests/test_flows/test_flow.py | 22 ++++++------- tests/test_runs/test_run_functions.py | 8 ++--- tests/test_setups/test_setup_functions.py | 4 +-- tests/test_study/test_study_functions.py | 5 +-- tests/test_tasks/test_clustering_task.py | 3 +- tests/test_tasks/test_task.py | 3 +- 7 files changed, 55 insertions(+), 30 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 8acc8ff38..b5932cf03 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -114,14 +114,37 @@ def tearDown(self): @classmethod def _track_test_server_dumps(self, entity_type, entity_id): + """ Static record of entities uploaded to test server + + Dictionary of lists where the keys are 'entity_type'. + Each such dictionary is a list of integer IDs. + For entity_type='flow', each list element is a tuple + of the form (Flow ID, Flow Name). + """ if entity_type not in TestBase.tracker: TestBase.tracker[entity_type] = [entity_id] else: TestBase.tracker[entity_type].append(entity_id) + @classmethod + def _delete_entity_from_tracker(self, entity_type, entity): + if entity_type in TestBase.tracker: + # delete_index handles duplicate entries + delete_index = [] + for i, element in enumerate(TestBase.tracker[entity_type]): + if entity_type == 'flow': + id, name = element + else: + id = element + if id == entity: + delete_index.append(i) + TestBase.tracker[entity_type] = [TestBase.tracker[entity_type][index] + for index in range(len(TestBase.tracker[entity_type])) + if index not in delete_index] + @pytest.fixture(scope="session", autouse=True) def _cleanup_fixture(self): - """Cleans up files generated by Unit tests + """Cleans up files generated by unit tests This function is called at the beginning of the invocation of TestBase (defined below), by each of class that inherits TestBase. @@ -149,7 +172,8 @@ def _cleanup_fixture(self): # pauses the code execution here till all tests in the 'session' is over yield # resumes from here after all collected tests are completed - # local file deletion + + # Local file deletion files = os.walk(directory) new_file_list = [] for root, _, filenames in files: @@ -170,26 +194,30 @@ def _cleanup_fixture(self): # putting 'run' in the start of the list entity_types[0], entity_types[index] = entity_types[index], entity_types[0] + # cloning file tracker to allow deletion of entries of deleted files + tracker = TestBase.tracker.copy() # reordering to delete sub flows later if 'flow' in entity_types: flows = {} - for entity_id, entity_name in TestBase.tracker['flow']: + for entity_id, entity_name in tracker['flow']: flows[entity_name] = entity_id # reordering flow names in descending order of their flow name lengths flow_deletion_order = [flows[name] for name in sorted(list(flows.keys()), key=lambda x: len(x), reverse=True)] - TestBase.tracker['flow'] = flow_deletion_order + tracker['flow'] = flow_deletion_order # deleting all collected entities published to test server for entity_type in entity_types: - for i, entity in enumerate(TestBase.tracker[entity_type]): + for i, entity in enumerate(tracker[entity_type]): try: openml.utils._delete_entity(entity_type, entity) print("Deleted ({}, {})".format(entity_type, entity)) - TestBase.tracker[entity_type].pop(i) + # deleting actual entry from tracker + TestBase._delete_entity_from_tracker(entity_type, entity) except Exception as e: print("Cannot delete ({}, {}): {}".format(entity_type, entity, e)) + print("End of cleanup_fixture from {}\n".format(self.__class__)) def _get_sentinel(self, sentinel=None): if sentinel is None: diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index e8dbecb9e..d43ddfcb7 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -180,7 +180,7 @@ def test_publish_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @@ -192,9 +192,8 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) - print(TestFlow.tracker) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -205,7 +204,7 @@ def test_publish_flow_with_similar_components(self): flow = self.extension.model_to_flow(clf) flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) # For a flow where both components are published together, the upload # date should be equal @@ -223,9 +222,8 @@ def test_publish_flow_with_similar_components(self): flow1 = self.extension.model_to_flow(clf1) flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() - self._track_test_server_dumps('flow', flow1.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow1.flow_id)) - print(TestFlow.tracker) # In order to assign different upload times to the flows! time.sleep(1) @@ -235,7 +233,7 @@ def test_publish_flow_with_similar_components(self): flow2 = self.extension.model_to_flow(clf2) flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() - self._track_test_server_dumps('flow', flow2.flow_id) + self._track_test_server_dumps('flow', (flow2.flow_id, flow2.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow2.flow_id)) # If one component was published before the other, the components in # the flow should have different upload dates @@ -249,7 +247,7 @@ def test_publish_flow_with_similar_components(self): # Child flow has different parameter. Check for storing the flow # correctly on the server should thus not check the child's parameters! flow3.publish() - self._track_test_server_dumps('flow', flow3.flow_id) + self._track_test_server_dumps('flow', (flow3.flow_id, flow3.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow3.flow_id)) def test_semi_legal_flow(self): @@ -263,7 +261,7 @@ def test_semi_legal_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) @mock.patch('openml.flows.functions.get_flow') @@ -292,7 +290,7 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) fixture = ( @@ -359,7 +357,7 @@ def test_existing_flow_exists(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) # publish the flow flow = flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) # redownload the flow flow = openml.flows.get_flow(flow.flow_id) @@ -419,7 +417,7 @@ def test_sklearn_to_upload_to_flow(self): flow, sentinel = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) diff --git a/tests/test_runs/test_run_functions.py b/tests/test_runs/test_run_functions.py index 337a05df6..23f17b552 100644 --- a/tests/test_runs/test_run_functions.py +++ b/tests/test_runs/test_run_functions.py @@ -184,7 +184,7 @@ def _remove_random_state(flow): flow, _ = self._add_sentinel_to_flow_name(flow, sentinel) if not openml.flows.flow_exists(flow.name, flow.external_version): flow.publish() - TestBase._track_test_server_dumps('flow', flow.flow_id) + TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from test_run_functions: {}".format(flow.flow_id)) task = openml.tasks.get_task(task_id) @@ -980,7 +980,7 @@ def test_run_with_illegal_flow_id_1(self): flow_orig = self.extension.model_to_flow(clf) try: flow_orig.publish() # ensures flow exist on server - TestBase._track_test_server_dumps('flow', flow_orig.flow_id) + TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) print("\ncollected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists @@ -1007,7 +1007,7 @@ def test_run_with_illegal_flow_id_1_after_load(self): flow_orig = self.extension.model_to_flow(clf) try: flow_orig.publish() # ensures flow exist on server - TestBase._track_test_server_dumps('flow', flow_orig.flow_id) + TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) print("\ncollected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists @@ -1281,7 +1281,7 @@ def test_run_flow_on_task_downloaded_flow(self): model = sklearn.ensemble.RandomForestClassifier(n_estimators=33) flow = self.extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) - TestBase._track_test_server_dumps('flow', flow.flow_id) + TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from test_run_functions: {}".format(flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id) diff --git a/tests/test_setups/test_setup_functions.py b/tests/test_setups/test_setup_functions.py index 8ce3da532..a93503d13 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -40,7 +40,7 @@ def test_nonexisting_setup_exists(self): flow = self.extension.model_to_flow(dectree) flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) # although the flow exists (created as of previous statement), @@ -54,7 +54,7 @@ def _existing_setup_exists(self, classif): flow = self.extension.model_to_flow(classif) flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() - self._track_test_server_dumps('flow', flow.flow_id) + self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) # although the flow exists, we can be sure there are no diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index c23b9776e..d47841d9b 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -135,8 +135,9 @@ def test_publish_study(self): run_ids=list(run_list.keys()) ) study_id = study.publish() - self._track_test_server_dumps('study', study_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], study_id)) + # not tracking upload for delete since _delete_entity called end of function + # self._track_test_server_dumps('study', study_id) + # print("\ncollected from {}: {}".format( __file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) study_downloaded = openml.study.get_study(study_id) self.assertEqual(study_downloaded.alias, fixt_alias) diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index 7c6e150b2..301749bb2 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -44,6 +44,5 @@ def test_upload_task(self): ) task_id = task.publish() - TestBase._track_test_server_dumps('task', task_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], task_id)) + # not tracking upload for delete since _delete_entity called end of function openml.utils._delete_entity('task', task_id) diff --git a/tests/test_tasks/test_task.py b/tests/test_tasks/test_task.py index d97f9cf09..563ca9ab0 100644 --- a/tests/test_tasks/test_task.py +++ b/tests/test_tasks/test_task.py @@ -59,8 +59,7 @@ def test_upload_task(self): ) task_id = task.publish() - TestBase._track_test_server_dumps('task', task_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], task_id)) + # not tracking upload for delete since _delete_entity called end of function # success break except OpenMLServerException as e: From f0951d48338ee24ea23fc31c79598d3fcde9485d Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Fri, 12 Jul 2019 13:40:40 +0200 Subject: [PATCH 10/17] Fixing unit test git status --- ci_scripts/test.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci_scripts/test.sh b/ci_scripts/test.sh index 51ecace49..9e7bc1326 100644 --- a/ci_scripts/test.sh +++ b/ci_scripts/test.sh @@ -3,6 +3,8 @@ set -e # check status and branch before running the unit tests before="`git status --porcelain -b`" before="$before" +# storing current working directory +curr_dir=`pwd` run_tests() { # Get into a temp directory to run test from the installed scikit learn and @@ -37,6 +39,8 @@ if [[ "$SKIP_TESTS" != "true" ]]; then run_tests fi +# changing directory to stored working directory +cd $curr_dir # check status and branch after running the unit tests # compares with $before to check for remaining files after="`git status --porcelain -b`" From 09b7f63f23f4579d4c82bd93012390132a0971da Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Fri, 12 Jul 2019 14:20:02 +0200 Subject: [PATCH 11/17] Fixing PEP8 issues --- openml/testing.py | 2 +- tests/test_datasets/test_dataset_functions.py | 24 +++++++++---------- .../test_sklearn_extension.py | 2 +- tests/test_flows/test_flow.py | 20 ++++++++-------- tests/test_flows/test_flow_functions.py | 5 ++-- tests/test_runs/test_run.py | 2 +- tests/test_setups/test_setup_functions.py | 6 ++--- tests/test_study/test_study_examples.py | 2 +- tests/test_study/test_study_functions.py | 6 ++--- tests/test_tasks/test_clustering_task.py | 1 - 10 files changed, 33 insertions(+), 37 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index 3ec4dd0ee..41052d8dd 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -28,7 +28,7 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ - tracker = {} + tracker: Dict[str, int] = {} test_server = None apikey = None diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index d2c8e091d..202361435 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -479,7 +479,7 @@ def test_publish_dataset(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], dataset.dataset_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test__retrieve_class_labels(self): @@ -501,7 +501,7 @@ def test_upload_dataset_with_url(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], dataset.dataset_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test_data_status(self): @@ -512,7 +512,7 @@ def test_data_status(self): url="https://www.openml.org/data/download/61/dataset_61_iris.arff") dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], dataset.dataset_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) did = dataset.dataset_id # admin key for test server (only adminds can activate datasets. @@ -627,7 +627,7 @@ def test_create_dataset_numpy(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), @@ -691,7 +691,7 @@ def test_create_dataset_list(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -736,7 +736,7 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -775,7 +775,7 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -900,7 +900,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -936,7 +936,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -974,7 +974,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) downloaded_data = _get_online_dataset_arff(upload_did) self.assertEqual( downloaded_data, @@ -1144,7 +1144,7 @@ def test___publish_fetch_ignore_attribute(self): # publish dataset upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) # test if publish was successful self.assertIsInstance(upload_did, int) # variables to carry forward for test_publish_fetch_ignore_attribute() @@ -1276,7 +1276,7 @@ def test_create_dataset_row_id_attribute_inference(self): self.assertEqual(dataset.row_id_attribute, output_row_id) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], upload_did)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) arff_dataset = arff.loads(_get_online_dataset_arff(upload_did)) arff_data = np.array(arff_dataset['data'], dtype=object) # if we set the name of the index then the index will be added to diff --git a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py index 85940f9bf..21dd3e112 100644 --- a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py +++ b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py @@ -1127,7 +1127,7 @@ def test_openml_param_name_to_sklearn(self): run = openml.runs.run_flow_on_task(flow, task) run = run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], run.run_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) run = openml.runs.get_run(run.run_id) setup = openml.setups.get_setup(run.setup_id) diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index d43ddfcb7..c5a33706e 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -181,7 +181,7 @@ def test_publish_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @mock.patch('openml.flows.functions.flow_exists') @@ -193,7 +193,7 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -205,7 +205,7 @@ def test_publish_flow_with_similar_components(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # For a flow where both components are published together, the upload # date should be equal self.assertEqual( @@ -223,7 +223,7 @@ def test_publish_flow_with_similar_components(self): flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow1.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) # In order to assign different upload times to the flows! time.sleep(1) @@ -234,7 +234,7 @@ def test_publish_flow_with_similar_components(self): flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() self._track_test_server_dumps('flow', (flow2.flow_id, flow2.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow2.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) # If one component was published before the other, the components in # the flow should have different upload dates self.assertNotEqual(flow2.upload_date, @@ -248,7 +248,7 @@ def test_publish_flow_with_similar_components(self): # correctly on the server should thus not check the child's parameters! flow3.publish() self._track_test_server_dumps('flow', (flow3.flow_id, flow3.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow3.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) def test_semi_legal_flow(self): # TODO: Test if parameters are set correctly! @@ -262,7 +262,7 @@ def test_semi_legal_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @mock.patch('openml.flows.functions.get_flow') @mock.patch('openml.flows.functions.flow_exists') @@ -291,7 +291,7 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) fixture = ( "Flow was not stored correctly on the server. " @@ -358,7 +358,7 @@ def test_existing_flow_exists(self): # publish the flow flow = flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # redownload the flow flow = openml.flows.get_flow(flow.flow_id) @@ -418,7 +418,7 @@ def test_sklearn_to_upload_to_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) # Check whether we can load the flow again diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index a1b45d586..1b391f4e0 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -7,7 +7,6 @@ from sklearn import ensemble import pandas as pd -import os import openml from openml.testing import TestBase import openml.extensions.sklearn @@ -259,7 +258,7 @@ def test_sklearn_to_flow_list_of_lists(self): self._add_sentinel_to_flow_name(flow) flow.publish() TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # Test deserialization works server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertEqual(server_flow.parameters['categories'], '[[0, 1], [0, 1]]') @@ -271,7 +270,7 @@ def test_get_flow_reinstantiate_model(self): flow = extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertIsInstance(downloaded_flow.model, sklearn.ensemble.RandomForestClassifier) diff --git a/tests/test_runs/test_run.py b/tests/test_runs/test_run.py index 3503803ad..ec7dc914d 100644 --- a/tests/test_runs/test_run.py +++ b/tests/test_runs/test_run.py @@ -231,7 +231,7 @@ def test_publish_with_local_loaded_flow(self): loaded_run = openml.runs.OpenMLRun.from_filesystem(cache_path) loaded_run.publish() TestBase._track_test_server_dumps('run', loaded_run.run_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], loaded_run.run_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], loaded_run.run_id)) # make sure the flow is published as part of publishing the run. self.assertTrue(openml.flows.flow_exists(flow.name, flow.external_version)) diff --git a/tests/test_setups/test_setup_functions.py b/tests/test_setups/test_setup_functions.py index a93503d13..63218b26f 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -41,7 +41,7 @@ def test_nonexisting_setup_exists(self): flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists (created as of previous statement), # we can be sure there are no setups (yet) as it was just created @@ -55,7 +55,7 @@ def _existing_setup_exists(self, classif): flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], flow.flow_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists, we can be sure there are no # setups (yet) as it hasn't been ran @@ -71,7 +71,7 @@ def _existing_setup_exists(self, classif): run.flow_id = flow.flow_id run.publish() self._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], run.run_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) # download the run, as it contains the right setup id run = openml.runs.get_run(run.run_id) diff --git a/tests/test_study/test_study_examples.py b/tests/test_study/test_study_examples.py index 477327ca8..f74706748 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -52,5 +52,5 @@ def test_Figure1a(self): print('Data set: %s; Accuracy: %0.2f' % (task.get_dataset().name, score.mean())) run.publish() # publish the experiment on OpenML (optional) self._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], run.run_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) print('URL for run: %s/run/%d' % (openml.config.server, run.run_id)) diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index d47841d9b..4b19638b4 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -78,7 +78,7 @@ def test_publish_benchmark_suite(self): ) study_id = study.publish() self._track_test_server_dumps('study', study_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], study_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) @@ -136,8 +136,6 @@ def test_publish_study(self): ) study_id = study.publish() # not tracking upload for delete since _delete_entity called end of function - # self._track_test_server_dumps('study', study_id) - # print("\ncollected from {}: {}".format( __file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) study_downloaded = openml.study.get_study(study_id) self.assertEqual(study_downloaded.alias, fixt_alias) @@ -188,7 +186,7 @@ def test_study_attach_illegal(self): ) study_id = study.publish() self._track_test_server_dumps('study', study_id) - print("\ncollected from {}: {}".format( __file__.split('/')[-1], study_id)) + print("\ncollected from {}: {}".format(__file__.split('/')[-1], study_id)) study_original = openml.study.get_study(study_id) with self.assertRaisesRegex(openml.exceptions.OpenMLServerException, diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index 301749bb2..85b701aee 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,5 +1,4 @@ import openml -from openml.testing import TestBase from .test_task import OpenMLTaskTest From 28a5f530f79c24be1e59513a140ff43441cab26f Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Fri, 12 Jul 2019 15:23:08 +0200 Subject: [PATCH 12/17] FIxing type annotation --- openml/testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openml/testing.py b/openml/testing.py index 41052d8dd..c0f42e86e 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -28,7 +28,7 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ - tracker: Dict[str, int] = {} + tracker = {} # type: dict test_server = None apikey = None From 5794ec8754543784205ca0e6bb669bb288181af0 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Tue, 16 Jul 2019 13:04:49 +0200 Subject: [PATCH 13/17] Logging and leaner flow --- openml/testing.py | 95 ++++++++++--------- tests/test_datasets/test_dataset_functions.py | 24 ++--- .../test_sklearn_extension.py | 2 +- tests/test_flows/test_flow.py | 20 ++-- tests/test_flows/test_flow_functions.py | 4 +- tests/test_runs/test_run.py | 6 +- tests/test_runs/test_run_functions.py | 22 ++--- tests/test_setups/test_setup_functions.py | 6 +- tests/test_study/test_study_examples.py | 2 +- tests/test_study/test_study_functions.py | 5 +- tests/test_tasks/test_clustering_task.py | 5 +- tests/test_tasks/test_task.py | 8 +- 12 files changed, 100 insertions(+), 99 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index c0f42e86e..d5366308d 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -18,6 +18,7 @@ from openml.tasks import TaskTypeEnum import pytest +import logging class TestBase(unittest.TestCase): @@ -28,9 +29,17 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ - tracker = {} # type: dict - test_server = None - apikey = None + publish_tracker = {} # type: dict + test_server = "https://test.openml.org/api/v1/xml" + # amueller's read/write key that he will throw away later + apikey = "610344db6388d9ba34f6db45a3cf71de" + + # creating logger for unit test file deletion status + logger = logging.getLogger("unit_tests") + logger.setLevel(logging.INFO) + fh = logging.FileHandler('TestBase.log') + fh.setLevel(logging.INFO) + logger.addHandler(fh) def setUp(self, n_levels: int = 1): """Setup variables and temporary directories. @@ -75,16 +84,9 @@ def setUp(self, n_levels: int = 1): os.chdir(self.workdir) self.cached = True - # amueller's read/write key that he will throw away later - openml.config.apikey = "610344db6388d9ba34f6db45a3cf71de" + openml.config.apikey = TestBase.apikey self.production_server = "https://openml.org/api/v1/xml" - self.test_server = "https://test.openml.org/api/v1/xml" - - # For global file deletion on test server - TestBase.test_server = self.test_server - TestBase.apikey = openml.config.apikey - - openml.config.server = self.test_server + openml.config.server = TestBase.test_server openml.config.avoid_duplicate_runs = False openml.config.cache_directory = self.workdir @@ -121,26 +123,28 @@ def _track_test_server_dumps(self, entity_type, entity_id): For entity_type='flow', each list element is a tuple of the form (Flow ID, Flow Name). """ - if entity_type not in TestBase.tracker: - TestBase.tracker[entity_type] = [entity_id] + if entity_type not in TestBase.publish_tracker: + TestBase.publish_tracker[entity_type] = [entity_id] else: - TestBase.tracker[entity_type].append(entity_id) + TestBase.publish_tracker[entity_type].append(entity_id) @classmethod def _delete_entity_from_tracker(self, entity_type, entity): - if entity_type in TestBase.tracker: - # delete_index handles duplicate entries - delete_index = [] - for i, element in enumerate(TestBase.tracker[entity_type]): - if entity_type == 'flow': - id, name = element - else: - id = element - if id == entity: - delete_index.append(i) - TestBase.tracker[entity_type] = [TestBase.tracker[entity_type][index] - for index in range(len(TestBase.tracker[entity_type])) - if index not in delete_index] + """ Deletes entity records from the static file_tracker + + Given an entity type and corresponding ID, deletes all entries, including + duplicate entries of the ID for the entity type. + """ + if entity_type in TestBase.publish_tracker: + # removes duplicate entries + TestBase.publish_tracker[entity_type] = list(set(TestBase.publish_tracker[entity_type])) + if entity_type == 'flow': + delete_index = [i for i, (id_, _) in enumerate(TestBase.publish_tracker[entity_type]) + if id_ == entity][0] + else: + delete_index = [i for i, id_ in enumerate(TestBase.publish_tracker[entity_type]) + if id_ == entity][0] + TestBase.publish_tracker[entity_type].pop(delete_index) @pytest.fixture(scope="session", autouse=True) def _cleanup_fixture(self): @@ -173,7 +177,9 @@ def _cleanup_fixture(self): yield # resumes from here after all collected tests are completed + # # Local file deletion + # files = os.walk(directory) new_file_list = [] for root, _, filenames in files: @@ -184,27 +190,24 @@ def _cleanup_fixture(self): for file in new_file_list: os.remove(file) + # # Test server deletion + # openml.config.server = TestBase.test_server openml.config.apikey = TestBase.apikey - entity_types = list(TestBase.tracker.keys()) - # deleting 'run' first to allow other dependent entities to be deleted - if 'run' in entity_types: - index = entity_types.index('run') - # putting 'run' in the start of the list - entity_types[0], entity_types[index] = entity_types[index], entity_types[0] + + # legal_entities defined in openml.utils._delete_entity + entity_types = {'run', 'data', 'flow', 'task', 'study', 'user'} + # 'run' needs to be first entity to allow other dependent entities to be deleted # cloning file tracker to allow deletion of entries of deleted files - tracker = TestBase.tracker.copy() - # reordering to delete sub flows later + tracker = TestBase.publish_tracker.copy() + + # reordering to delete sub flows at the end of flows + # sub-flows have shorter names, hence, sorting by descending order of flow name length if 'flow' in entity_types: - flows = {} - for entity_id, entity_name in tracker['flow']: - flows[entity_name] = entity_id - # reordering flows in descending order of their flow name lengths - flow_deletion_order = [flows[name] for name in sorted(list(flows.keys()), - key=lambda x: len(x), - reverse=True)] + flow_deletion_order = [entity_id for entity_id, _ in + sorted(tracker['flow'], key=lambda x: len(x[1]), reverse=True)] tracker['flow'] = flow_deletion_order # deleting all collected entities published to test server @@ -212,12 +215,12 @@ def _cleanup_fixture(self): for i, entity in enumerate(tracker[entity_type]): try: openml.utils._delete_entity(entity_type, entity) - print("Deleted ({}, {})".format(entity_type, entity)) + TestBase.logger.info("Deleted ({}, {})".format(entity_type, entity)) # deleting actual entry from tracker TestBase._delete_entity_from_tracker(entity_type, entity) except Exception as e: - print("Cannot delete ({}, {}): {}".format(entity_type, entity, e)) - print("End of cleanup_fixture from {}\n".format(self.__class__)) + TestBase.logger.warn("Cannot delete ({}, {}): {}".format(entity_type, entity, e)) + TestBase.logger.info("End of cleanup_fixture from {}".format(self.__class__)) def _get_sentinel(self, sentinel=None): if sentinel is None: diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index 202361435..aecdeaf65 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -479,7 +479,7 @@ def test_publish_dataset(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test__retrieve_class_labels(self): @@ -501,7 +501,7 @@ def test_upload_dataset_with_url(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test_data_status(self): @@ -512,7 +512,7 @@ def test_data_status(self): url="https://www.openml.org/data/download/61/dataset_61_iris.arff") dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) did = dataset.dataset_id # admin key for test server (only adminds can activate datasets. @@ -627,7 +627,7 @@ def test_create_dataset_numpy(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), @@ -691,7 +691,7 @@ def test_create_dataset_list(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -736,7 +736,7 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -775,7 +775,7 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -900,7 +900,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -936,7 +936,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -974,7 +974,7 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) downloaded_data = _get_online_dataset_arff(upload_did) self.assertEqual( downloaded_data, @@ -1144,7 +1144,7 @@ def test___publish_fetch_ignore_attribute(self): # publish dataset upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) # test if publish was successful self.assertIsInstance(upload_did, int) # variables to carry forward for test_publish_fetch_ignore_attribute() @@ -1276,7 +1276,7 @@ def test_create_dataset_row_id_attribute_inference(self): self.assertEqual(dataset.row_id_attribute, output_row_id) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) arff_dataset = arff.loads(_get_online_dataset_arff(upload_did)) arff_data = np.array(arff_dataset['data'], dtype=object) # if we set the name of the index then the index will be added to diff --git a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py index 21dd3e112..4f198aff4 100644 --- a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py +++ b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py @@ -1127,7 +1127,7 @@ def test_openml_param_name_to_sklearn(self): run = openml.runs.run_flow_on_task(flow, task) run = run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) run = openml.runs.get_run(run.run_id) setup = openml.setups.get_setup(run.setup_id) diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index c5a33706e..7a7f7309d 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -181,7 +181,7 @@ def test_publish_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @mock.patch('openml.flows.functions.flow_exists') @@ -193,7 +193,7 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -205,7 +205,7 @@ def test_publish_flow_with_similar_components(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # For a flow where both components are published together, the upload # date should be equal self.assertEqual( @@ -223,7 +223,7 @@ def test_publish_flow_with_similar_components(self): flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) # In order to assign different upload times to the flows! time.sleep(1) @@ -234,7 +234,7 @@ def test_publish_flow_with_similar_components(self): flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() self._track_test_server_dumps('flow', (flow2.flow_id, flow2.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) # If one component was published before the other, the components in # the flow should have different upload dates self.assertNotEqual(flow2.upload_date, @@ -248,7 +248,7 @@ def test_publish_flow_with_similar_components(self): # correctly on the server should thus not check the child's parameters! flow3.publish() self._track_test_server_dumps('flow', (flow3.flow_id, flow3.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) def test_semi_legal_flow(self): # TODO: Test if parameters are set correctly! @@ -262,7 +262,7 @@ def test_semi_legal_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @mock.patch('openml.flows.functions.get_flow') @mock.patch('openml.flows.functions.flow_exists') @@ -291,7 +291,7 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) fixture = ( "Flow was not stored correctly on the server. " @@ -358,7 +358,7 @@ def test_existing_flow_exists(self): # publish the flow flow = flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # redownload the flow flow = openml.flows.get_flow(flow.flow_id) @@ -418,7 +418,7 @@ def test_sklearn_to_upload_to_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) # Check whether we can load the flow again diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index 1b391f4e0..f78060b06 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -258,7 +258,7 @@ def test_sklearn_to_flow_list_of_lists(self): self._add_sentinel_to_flow_name(flow) flow.publish() TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # Test deserialization works server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertEqual(server_flow.parameters['categories'], '[[0, 1], [0, 1]]') @@ -270,7 +270,7 @@ def test_get_flow_reinstantiate_model(self): flow = extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) self.assertIsInstance(downloaded_flow.model, sklearn.ensemble.RandomForestClassifier) diff --git a/tests/test_runs/test_run.py b/tests/test_runs/test_run.py index ec7dc914d..fd74af9fd 100644 --- a/tests/test_runs/test_run.py +++ b/tests/test_runs/test_run.py @@ -130,7 +130,7 @@ def test_to_from_filesystem_vanilla(self): self._test_run_obj_equals(run, run_prime) run_prime.publish() TestBase._track_test_server_dumps('run', run_prime.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) def test_to_from_filesystem_search(self): @@ -165,7 +165,7 @@ def test_to_from_filesystem_search(self): self._test_run_obj_equals(run, run_prime) run_prime.publish() TestBase._track_test_server_dumps('run', run_prime.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) def test_to_from_filesystem_no_model(self): @@ -231,7 +231,7 @@ def test_publish_with_local_loaded_flow(self): loaded_run = openml.runs.OpenMLRun.from_filesystem(cache_path) loaded_run.publish() TestBase._track_test_server_dumps('run', loaded_run.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], loaded_run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], loaded_run.run_id)) # make sure the flow is published as part of publishing the run. self.assertTrue(openml.flows.flow_exists(flow.name, flow.external_version)) diff --git a/tests/test_runs/test_run_functions.py b/tests/test_runs/test_run_functions.py index 23f17b552..86953fade 100644 --- a/tests/test_runs/test_run_functions.py +++ b/tests/test_runs/test_run_functions.py @@ -185,7 +185,7 @@ def _remove_random_state(flow): if not openml.flows.flow_exists(flow.name, flow.external_version): flow.publish() TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from test_run_functions: {}".format(flow.flow_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(flow.flow_id)) task = openml.tasks.get_task(task_id) @@ -199,7 +199,7 @@ def _remove_random_state(flow): ) run_ = run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from test_run_functions: {}".format(run.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) self.assertEqual(run_, run) self.assertIsInstance(run.dataset_id, int) @@ -692,7 +692,7 @@ def test_initialize_cv_from_run(self): ) run_ = run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from test_run_functions: {}".format(run.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) run = openml.runs.get_run(run_.run_id) modelR = openml.runs.initialize_model_from_run(run_id=run.run_id) @@ -809,7 +809,7 @@ def test_initialize_model_from_run(self): ) run_ = run.publish() TestBase._track_test_server_dumps('run', run_.run_id) - print("\ncollected from test_run_functions: {}".format(run_.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(run_.run_id)) run = openml.runs.get_run(run_.run_id) modelR = openml.runs.initialize_model_from_run(run_id=run.run_id) @@ -862,7 +862,7 @@ def test_get_run_trace(self): ) run = run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from test_run_functions: {}".format(run.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) self._wait_for_processed_run(run.run_id, 200) run_id = run.run_id except openml.exceptions.OpenMLRunsExistError as e: @@ -908,7 +908,7 @@ def test__run_exists(self): ) run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from test_run_functions: {}".format(run.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) except openml.exceptions.PyOpenMLError: # run already existed. Great. pass @@ -970,7 +970,7 @@ def test_run_with_illegal_flow_id_after_load(self): with self.assertRaisesRegex(openml.exceptions.PyOpenMLError, expected_message_regex): loaded_run.publish() TestBase._track_test_server_dumps('run', loaded_run.run_id) - print("\ncollected from test_run_functions: {}".format(loaded_run.run_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(loaded_run.run_id)) def test_run_with_illegal_flow_id_1(self): # Check the case where the user adds an illegal flow id to an existing @@ -981,7 +981,7 @@ def test_run_with_illegal_flow_id_1(self): try: flow_orig.publish() # ensures flow exist on server TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) - print("\ncollected from test_run_functions: {}".format(flow_orig.flow_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists pass @@ -1008,7 +1008,7 @@ def test_run_with_illegal_flow_id_1_after_load(self): try: flow_orig.publish() # ensures flow exist on server TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) - print("\ncollected from test_run_functions: {}".format(flow_orig.flow_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists pass @@ -1282,7 +1282,7 @@ def test_run_flow_on_task_downloaded_flow(self): flow = self.extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from test_run_functions: {}".format(flow.flow_id)) + TestBase.logger.info("collected from test_run_functions: {}".format(flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id) task = openml.tasks.get_task(119) # diabetes @@ -1295,4 +1295,4 @@ def test_run_flow_on_task_downloaded_flow(self): run.publish() TestBase._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) diff --git a/tests/test_setups/test_setup_functions.py b/tests/test_setups/test_setup_functions.py index 63218b26f..016d323b4 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -41,7 +41,7 @@ def test_nonexisting_setup_exists(self): flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists (created as of previous statement), # we can be sure there are no setups (yet) as it was just created @@ -55,7 +55,7 @@ def _existing_setup_exists(self, classif): flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists, we can be sure there are no # setups (yet) as it hasn't been ran @@ -71,7 +71,7 @@ def _existing_setup_exists(self, classif): run.flow_id = flow.flow_id run.publish() self._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) # download the run, as it contains the right setup id run = openml.runs.get_run(run.run_id) diff --git a/tests/test_study/test_study_examples.py b/tests/test_study/test_study_examples.py index f74706748..2822f682b 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -52,5 +52,5 @@ def test_Figure1a(self): print('Data set: %s; Accuracy: %0.2f' % (task.get_dataset().name, score.mean())) run.publish() # publish the experiment on OpenML (optional) self._track_test_server_dumps('run', run.run_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) print('URL for run: %s/run/%d' % (openml.config.server, run.run_id)) diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index 4b19638b4..90f879223 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -78,7 +78,7 @@ def test_publish_benchmark_suite(self): ) study_id = study.publish() self._track_test_server_dumps('study', study_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], study_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) @@ -136,6 +136,7 @@ def test_publish_study(self): ) study_id = study.publish() # not tracking upload for delete since _delete_entity called end of function + # asserting return status from openml.study.delete_study() self.assertGreater(study_id, 0) study_downloaded = openml.study.get_study(study_id) self.assertEqual(study_downloaded.alias, fixt_alias) @@ -186,7 +187,7 @@ def test_study_attach_illegal(self): ) study_id = study.publish() self._track_test_server_dumps('study', study_id) - print("\ncollected from {}: {}".format(__file__.split('/')[-1], study_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) study_original = openml.study.get_study(study_id) with self.assertRaisesRegex(openml.exceptions.OpenMLServerException, diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index 85b701aee..e8b8fa99f 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,4 +1,5 @@ import openml +from openml.testing import TestBase from .test_task import OpenMLTaskTest @@ -43,5 +44,5 @@ def test_upload_task(self): ) task_id = task.publish() - # not tracking upload for delete since _delete_entity called end of function - openml.utils._delete_entity('task', task_id) + TestBase._track_test_server_dumps('task', task_id) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], task_id)) diff --git a/tests/test_tasks/test_task.py b/tests/test_tasks/test_task.py index 563ca9ab0..0dfe41eac 100644 --- a/tests/test_tasks/test_task.py +++ b/tests/test_tasks/test_task.py @@ -11,9 +11,6 @@ create_task, get_task ) -from openml.utils import ( - _delete_entity, -) class OpenMLTaskTest(TestBase): @@ -59,7 +56,8 @@ def test_upload_task(self): ) task_id = task.publish() - # not tracking upload for delete since _delete_entity called end of function + TestBase._track_test_server_dumps('task', task_id) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], task_id)) # success break except OpenMLServerException as e: @@ -75,8 +73,6 @@ def test_upload_task(self): 'Could not create a valid task for task type ID {}'.format(self.task_type_id) ) - _delete_entity('task', task_id) - def _get_compatible_rand_dataset(self) -> int: compatible_datasets = [] From 3085c155ea1564c2baa5f234ce7441fedc8431ee Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Tue, 16 Jul 2019 17:02:38 +0200 Subject: [PATCH 14/17] Fixing PEP8 and unit test errors --- openml/testing.py | 13 +++---- tests/test_datasets/test_dataset_functions.py | 36 ++++++++++++------- tests/test_flows/test_flow.py | 30 ++++++++++------ tests/test_runs/test_run.py | 9 +++-- tests/test_study/test_study_examples.py | 3 +- tests/test_tasks/test_task.py | 3 +- 6 files changed, 61 insertions(+), 33 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index d5366308d..b51a4eab1 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -139,10 +139,12 @@ def _delete_entity_from_tracker(self, entity_type, entity): # removes duplicate entries TestBase.publish_tracker[entity_type] = list(set(TestBase.publish_tracker[entity_type])) if entity_type == 'flow': - delete_index = [i for i, (id_, _) in enumerate(TestBase.publish_tracker[entity_type]) + delete_index = [i for i, (id_, _) in + enumerate(TestBase.publish_tracker[entity_type]) if id_ == entity][0] else: - delete_index = [i for i, id_ in enumerate(TestBase.publish_tracker[entity_type]) + delete_index = [i for i, id_ in + enumerate(TestBase.publish_tracker[entity_type]) if id_ == entity][0] TestBase.publish_tracker[entity_type].pop(delete_index) @@ -196,9 +198,8 @@ def _cleanup_fixture(self): openml.config.server = TestBase.test_server openml.config.apikey = TestBase.apikey - - # legal_entities defined in openml.utils._delete_entity - entity_types = {'run', 'data', 'flow', 'task', 'study', 'user'} + # legal_entities defined in openml.utils._delete_entity - {'user'} + entity_types = {'run', 'data', 'flow', 'task', 'study'} # 'run' needs to be first entity to allow other dependent entities to be deleted # cloning file tracker to allow deletion of entries of deleted files tracker = TestBase.publish_tracker.copy() @@ -219,7 +220,7 @@ def _cleanup_fixture(self): # deleting actual entry from tracker TestBase._delete_entity_from_tracker(entity_type, entity) except Exception as e: - TestBase.logger.warn("Cannot delete ({}, {}): {}".format(entity_type, entity, e)) + TestBase.logger.warn("Cannot delete ({},{}): {}".format(entity_type, entity, e)) TestBase.logger.info("End of cleanup_fixture from {}".format(self.__class__)) def _get_sentinel(self, sentinel=None): diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index aecdeaf65..cc042a2ae 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -479,7 +479,8 @@ def test_publish_dataset(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test__retrieve_class_labels(self): @@ -501,7 +502,8 @@ def test_upload_dataset_with_url(self): ) dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) def test_data_status(self): @@ -512,7 +514,8 @@ def test_data_status(self): url="https://www.openml.org/data/download/61/dataset_61_iris.arff") dataset.publish() TestBase._track_test_server_dumps('data', dataset.dataset_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + dataset.dataset_id)) did = dataset.dataset_id # admin key for test server (only adminds can activate datasets. @@ -627,7 +630,8 @@ def test_create_dataset_numpy(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), @@ -691,7 +695,8 @@ def test_create_dataset_list(self): upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -736,7 +741,8 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -775,7 +781,8 @@ def test_create_dataset_sparse(self): upload_did = xor_dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -900,7 +907,8 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -936,7 +944,8 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -974,7 +983,8 @@ def test_create_dataset_pandas(self): ) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) downloaded_data = _get_online_dataset_arff(upload_did) self.assertEqual( downloaded_data, @@ -1144,7 +1154,8 @@ def test___publish_fetch_ignore_attribute(self): # publish dataset upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) # test if publish was successful self.assertIsInstance(upload_did, int) # variables to carry forward for test_publish_fetch_ignore_attribute() @@ -1276,7 +1287,8 @@ def test_create_dataset_row_id_attribute_inference(self): self.assertEqual(dataset.row_id_attribute, output_row_id) upload_did = dataset.publish() TestBase._track_test_server_dumps('data', upload_did) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) arff_dataset = arff.loads(_get_online_dataset_arff(upload_did)) arff_data = np.array(arff_dataset['data'], dtype=object) # if we set the name of the index then the index will be added to diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index 7a7f7309d..df242bb08 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -181,7 +181,8 @@ def test_publish_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @mock.patch('openml.flows.functions.flow_exists') @@ -193,7 +194,8 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -205,7 +207,8 @@ def test_publish_flow_with_similar_components(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) # For a flow where both components are published together, the upload # date should be equal self.assertEqual( @@ -223,7 +226,8 @@ def test_publish_flow_with_similar_components(self): flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow1.flow_id)) # In order to assign different upload times to the flows! time.sleep(1) @@ -234,7 +238,8 @@ def test_publish_flow_with_similar_components(self): flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() self._track_test_server_dumps('flow', (flow2.flow_id, flow2.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow2.flow_id)) # If one component was published before the other, the components in # the flow should have different upload dates self.assertNotEqual(flow2.upload_date, @@ -248,7 +253,8 @@ def test_publish_flow_with_similar_components(self): # correctly on the server should thus not check the child's parameters! flow3.publish() self._track_test_server_dumps('flow', (flow3.flow_id, flow3.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow3.flow_id)) def test_semi_legal_flow(self): # TODO: Test if parameters are set correctly! @@ -262,7 +268,8 @@ def test_semi_legal_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) @mock.patch('openml.flows.functions.get_flow') @mock.patch('openml.flows.functions.flow_exists') @@ -291,7 +298,8 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) fixture = ( "Flow was not stored correctly on the server. " @@ -358,7 +366,8 @@ def test_existing_flow_exists(self): # publish the flow flow = flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) # redownload the flow flow = openml.flows.get_flow(flow.flow_id) @@ -418,7 +427,8 @@ def test_sklearn_to_upload_to_flow(self): flow.publish() self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) self.assertIsInstance(flow.flow_id, int) # Check whether we can load the flow again diff --git a/tests/test_runs/test_run.py b/tests/test_runs/test_run.py index fd74af9fd..98a50b315 100644 --- a/tests/test_runs/test_run.py +++ b/tests/test_runs/test_run.py @@ -130,7 +130,8 @@ def test_to_from_filesystem_vanilla(self): self._test_run_obj_equals(run, run_prime) run_prime.publish() TestBase._track_test_server_dumps('run', run_prime.run_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + run_prime.run_id)) def test_to_from_filesystem_search(self): @@ -165,7 +166,8 @@ def test_to_from_filesystem_search(self): self._test_run_obj_equals(run, run_prime) run_prime.publish() TestBase._track_test_server_dumps('run', run_prime.run_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + run_prime.run_id)) def test_to_from_filesystem_no_model(self): @@ -231,7 +233,8 @@ def test_publish_with_local_loaded_flow(self): loaded_run = openml.runs.OpenMLRun.from_filesystem(cache_path) loaded_run.publish() TestBase._track_test_server_dumps('run', loaded_run.run_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], loaded_run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + loaded_run.run_id)) # make sure the flow is published as part of publishing the run. self.assertTrue(openml.flows.flow_exists(flow.name, flow.external_version)) diff --git a/tests/test_study/test_study_examples.py b/tests/test_study/test_study_examples.py index 2822f682b..ef92c5c4b 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -52,5 +52,6 @@ def test_Figure1a(self): print('Data set: %s; Accuracy: %0.2f' % (task.get_dataset().name, score.mean())) run.publish() # publish the experiment on OpenML (optional) self._track_test_server_dumps('run', run.run_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + run.run_id)) print('URL for run: %s/run/%d' % (openml.config.server, run.run_id)) diff --git a/tests/test_tasks/test_task.py b/tests/test_tasks/test_task.py index 0dfe41eac..9230afbaf 100644 --- a/tests/test_tasks/test_task.py +++ b/tests/test_tasks/test_task.py @@ -57,7 +57,8 @@ def test_upload_task(self): task_id = task.publish() TestBase._track_test_server_dumps('task', task_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], task_id)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + task_id)) # success break except OpenMLServerException as e: From 28dcb02cb086de26ad39f56fb8b0df0fef50ebc7 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Wed, 17 Jul 2019 19:52:42 +0200 Subject: [PATCH 15/17] Fixing test cases; Renaming function --- openml/testing.py | 7 ++- tests/test_datasets/test_dataset_functions.py | 24 +++++----- .../test_sklearn_extension.py | 2 +- tests/test_flows/test_flow.py | 20 ++++---- tests/test_flows/test_flow_functions.py | 4 +- tests/test_runs/test_run.py | 6 +-- tests/test_runs/test_run_functions.py | 22 ++++----- tests/test_setups/test_setup_functions.py | 6 +-- tests/test_study/test_study_examples.py | 2 +- tests/test_study/test_study_functions.py | 4 +- tests/test_tasks/test_clustering_task.py | 46 ++++++++++++------- tests/test_tasks/test_task.py | 2 +- 12 files changed, 80 insertions(+), 65 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index b51a4eab1..f15b85958 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -29,7 +29,10 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ - publish_tracker = {} # type: dict + publish_tracker = {'run': [], 'data': [], 'flow': [], 'task': [], + 'study': [], 'user': []} # type: dict + # legal_entities defined in openml.utils._delete_entity - {'user'} + entity_types = {'run', 'data', 'flow', 'task', 'study'} test_server = "https://test.openml.org/api/v1/xml" # amueller's read/write key that he will throw away later apikey = "610344db6388d9ba34f6db45a3cf71de" @@ -115,7 +118,7 @@ def tearDown(self): openml.config.connection_n_retries = self.connection_n_retries @classmethod - def _track_test_server_dumps(self, entity_type, entity_id): + def _mark_entity_for_removal(self, entity_type, entity_id): """ Static record of entities uploaded to test server Dictionary of lists where the keys are 'entity_type'. diff --git a/tests/test_datasets/test_dataset_functions.py b/tests/test_datasets/test_dataset_functions.py index cc042a2ae..80d7333a0 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -478,7 +478,7 @@ def test_publish_dataset(self): data_file=file_path, ) dataset.publish() - TestBase._track_test_server_dumps('data', dataset.dataset_id) + TestBase._mark_entity_for_removal('data', dataset.dataset_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) @@ -501,7 +501,7 @@ def test_upload_dataset_with_url(self): url="https://www.openml.org/data/download/61/dataset_61_iris.arff", ) dataset.publish() - TestBase._track_test_server_dumps('data', dataset.dataset_id) + TestBase._mark_entity_for_removal('data', dataset.dataset_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) self.assertIsInstance(dataset.dataset_id, int) @@ -513,7 +513,7 @@ def test_data_status(self): version=1, url="https://www.openml.org/data/download/61/dataset_61_iris.arff") dataset.publish() - TestBase._track_test_server_dumps('data', dataset.dataset_id) + TestBase._mark_entity_for_removal('data', dataset.dataset_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], dataset.dataset_id)) did = dataset.dataset_id @@ -629,7 +629,7 @@ def test_create_dataset_numpy(self): ) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) @@ -694,7 +694,7 @@ def test_create_dataset_list(self): ) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( @@ -740,7 +740,7 @@ def test_create_dataset_sparse(self): ) upload_did = xor_dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( @@ -780,7 +780,7 @@ def test_create_dataset_sparse(self): ) upload_did = xor_dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( @@ -906,7 +906,7 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( @@ -943,7 +943,7 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) self.assertEqual( @@ -982,7 +982,7 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) downloaded_data = _get_online_dataset_arff(upload_did) @@ -1153,7 +1153,7 @@ def test___publish_fetch_ignore_attribute(self): # publish dataset upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) # test if publish was successful @@ -1286,7 +1286,7 @@ def test_create_dataset_row_id_attribute_inference(self): ) self.assertEqual(dataset.row_id_attribute, output_row_id) upload_did = dataset.publish() - TestBase._track_test_server_dumps('data', upload_did) + TestBase._mark_entity_for_removal('data', upload_did) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], upload_did)) arff_dataset = arff.loads(_get_online_dataset_arff(upload_did)) diff --git a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py index 4f198aff4..2217b332b 100644 --- a/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py +++ b/tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py @@ -1126,7 +1126,7 @@ def test_openml_param_name_to_sklearn(self): task = openml.tasks.get_task(115) run = openml.runs.run_flow_on_task(flow, task) run = run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) run = openml.runs.get_run(run.run_id) setup = openml.setups.get_setup(run.setup_id) diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index df242bb08..9f36a50f0 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -180,7 +180,7 @@ def test_publish_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @@ -193,7 +193,7 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -206,7 +206,7 @@ def test_publish_flow_with_similar_components(self): flow = self.extension.model_to_flow(clf) flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # For a flow where both components are published together, the upload @@ -225,7 +225,7 @@ def test_publish_flow_with_similar_components(self): flow1 = self.extension.model_to_flow(clf1) flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) @@ -237,7 +237,7 @@ def test_publish_flow_with_similar_components(self): flow2 = self.extension.model_to_flow(clf2) flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() - self._track_test_server_dumps('flow', (flow2.flow_id, flow2.name)) + self._mark_entity_for_removal('flow', (flow2.flow_id, flow2.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) # If one component was published before the other, the components in @@ -252,7 +252,7 @@ def test_publish_flow_with_similar_components(self): # Child flow has different parameter. Check for storing the flow # correctly on the server should thus not check the child's parameters! flow3.publish() - self._track_test_server_dumps('flow', (flow3.flow_id, flow3.name)) + self._mark_entity_for_removal('flow', (flow3.flow_id, flow3.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) @@ -267,7 +267,7 @@ def test_semi_legal_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -297,7 +297,7 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -365,7 +365,7 @@ def test_existing_flow_exists(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) # publish the flow flow = flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # redownload the flow @@ -426,7 +426,7 @@ def test_sklearn_to_upload_to_flow(self): flow, sentinel = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) diff --git a/tests/test_flows/test_flow_functions.py b/tests/test_flows/test_flow_functions.py index f78060b06..02d4b2a7d 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -257,7 +257,7 @@ def test_sklearn_to_flow_list_of_lists(self): # Test flow is accepted by server self._add_sentinel_to_flow_name(flow) flow.publish() - TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # Test deserialization works server_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) @@ -269,7 +269,7 @@ def test_get_flow_reinstantiate_model(self): extension = openml.extensions.get_extension_by_model(model) flow = extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) - TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id, reinstantiate=True) diff --git a/tests/test_runs/test_run.py b/tests/test_runs/test_run.py index 98a50b315..d0c04fb9b 100644 --- a/tests/test_runs/test_run.py +++ b/tests/test_runs/test_run.py @@ -129,7 +129,7 @@ def test_to_from_filesystem_vanilla(self): self.assertTrue(run_prime.flow is None) self._test_run_obj_equals(run, run_prime) run_prime.publish() - TestBase._track_test_server_dumps('run', run_prime.run_id) + TestBase._mark_entity_for_removal('run', run_prime.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) @@ -165,7 +165,7 @@ def test_to_from_filesystem_search(self): run_prime = openml.runs.OpenMLRun.from_filesystem(cache_path) self._test_run_obj_equals(run, run_prime) run_prime.publish() - TestBase._track_test_server_dumps('run', run_prime.run_id) + TestBase._mark_entity_for_removal('run', run_prime.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) @@ -232,7 +232,7 @@ def test_publish_with_local_loaded_flow(self): # obtain run from filesystem loaded_run = openml.runs.OpenMLRun.from_filesystem(cache_path) loaded_run.publish() - TestBase._track_test_server_dumps('run', loaded_run.run_id) + TestBase._mark_entity_for_removal('run', loaded_run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], loaded_run.run_id)) diff --git a/tests/test_runs/test_run_functions.py b/tests/test_runs/test_run_functions.py index 86953fade..bd123cd37 100644 --- a/tests/test_runs/test_run_functions.py +++ b/tests/test_runs/test_run_functions.py @@ -184,7 +184,7 @@ def _remove_random_state(flow): flow, _ = self._add_sentinel_to_flow_name(flow, sentinel) if not openml.flows.flow_exists(flow.name, flow.external_version): flow.publish() - TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from test_run_functions: {}".format(flow.flow_id)) task = openml.tasks.get_task(task_id) @@ -198,7 +198,7 @@ def _remove_random_state(flow): avoid_duplicate_runs=openml.config.avoid_duplicate_runs, ) run_ = run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) self.assertEqual(run_, run) self.assertIsInstance(run.dataset_id, int) @@ -691,7 +691,7 @@ def test_initialize_cv_from_run(self): seed=1, ) run_ = run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) run = openml.runs.get_run(run_.run_id) @@ -808,7 +808,7 @@ def test_initialize_model_from_run(self): avoid_duplicate_runs=False, ) run_ = run.publish() - TestBase._track_test_server_dumps('run', run_.run_id) + TestBase._mark_entity_for_removal('run', run_.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(run_.run_id)) run = openml.runs.get_run(run_.run_id) @@ -861,7 +861,7 @@ def test_get_run_trace(self): num_iterations * num_folds, ) run = run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) self._wait_for_processed_run(run.run_id, 200) run_id = run.run_id @@ -907,7 +907,7 @@ def test__run_exists(self): upload_flow=True ) run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(run.run_id)) except openml.exceptions.PyOpenMLError: # run already existed. Great. @@ -969,7 +969,7 @@ def test_run_with_illegal_flow_id_after_load(self): "but 'flow.flow_id' is not None.") with self.assertRaisesRegex(openml.exceptions.PyOpenMLError, expected_message_regex): loaded_run.publish() - TestBase._track_test_server_dumps('run', loaded_run.run_id) + TestBase._mark_entity_for_removal('run', loaded_run.run_id) TestBase.logger.info("collected from test_run_functions: {}".format(loaded_run.run_id)) def test_run_with_illegal_flow_id_1(self): @@ -980,7 +980,7 @@ def test_run_with_illegal_flow_id_1(self): flow_orig = self.extension.model_to_flow(clf) try: flow_orig.publish() # ensures flow exist on server - TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) + TestBase._mark_entity_for_removal('flow', (flow_orig.flow_id, flow_orig.name)) TestBase.logger.info("collected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists @@ -1007,7 +1007,7 @@ def test_run_with_illegal_flow_id_1_after_load(self): flow_orig = self.extension.model_to_flow(clf) try: flow_orig.publish() # ensures flow exist on server - TestBase._track_test_server_dumps('flow', (flow_orig.flow_id, flow_orig.name)) + TestBase._mark_entity_for_removal('flow', (flow_orig.flow_id, flow_orig.name)) TestBase.logger.info("collected from test_run_functions: {}".format(flow_orig.flow_id)) except openml.exceptions.OpenMLServerException: # flow already exists @@ -1281,7 +1281,7 @@ def test_run_flow_on_task_downloaded_flow(self): model = sklearn.ensemble.RandomForestClassifier(n_estimators=33) flow = self.extension.model_to_flow(model) flow.publish(raise_error_if_exists=False) - TestBase._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from test_run_functions: {}".format(flow.flow_id)) downloaded_flow = openml.flows.get_flow(flow.flow_id) @@ -1294,5 +1294,5 @@ def test_run_flow_on_task_downloaded_flow(self): ) run.publish() - TestBase._track_test_server_dumps('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) diff --git a/tests/test_setups/test_setup_functions.py b/tests/test_setups/test_setup_functions.py index 016d323b4..4c15830cf 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -40,7 +40,7 @@ def test_nonexisting_setup_exists(self): flow = self.extension.model_to_flow(dectree) flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists (created as of previous statement), @@ -54,7 +54,7 @@ def _existing_setup_exists(self, classif): flow = self.extension.model_to_flow(classif) flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() - self._track_test_server_dumps('flow', (flow.flow_id, flow.name)) + self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists, we can be sure there are no @@ -70,7 +70,7 @@ def _existing_setup_exists(self, classif): # spoof flow id, otherwise the sentinel is ignored run.flow_id = flow.flow_id run.publish() - self._track_test_server_dumps('run', run.run_id) + self._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) # download the run, as it contains the right setup id run = openml.runs.get_run(run.run_id) diff --git a/tests/test_study/test_study_examples.py b/tests/test_study/test_study_examples.py index ef92c5c4b..fd9cf93f1 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -51,7 +51,7 @@ def test_Figure1a(self): ) # print accuracy score print('Data set: %s; Accuracy: %0.2f' % (task.get_dataset().name, score.mean())) run.publish() # publish the experiment on OpenML (optional) - self._track_test_server_dumps('run', run.run_id) + self._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) print('URL for run: %s/run/%d' % (openml.config.server, run.run_id)) diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index 90f879223..99787ca83 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -77,7 +77,7 @@ def test_publish_benchmark_suite(self): task_ids=fixture_task_ids ) study_id = study.publish() - self._track_test_server_dumps('study', study_id) + self._mark_entity_for_removal('study', study_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) @@ -186,7 +186,7 @@ def test_study_attach_illegal(self): run_ids=list(run_list.keys()) ) study_id = study.publish() - self._track_test_server_dumps('study', study_id) + self._mark_entity_for_removal('study', study_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) study_original = openml.study.get_study(study_id) diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index e8b8fa99f..b09eff7dc 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,6 +1,7 @@ import openml from openml.testing import TestBase from .test_task import OpenMLTaskTest +from openml.exceptions import OpenMLServerException class OpenMLClusteringTaskTest(OpenMLTaskTest): @@ -29,20 +30,31 @@ def test_download_task(self): self.assertEqual(task.dataset_id, 36) def test_upload_task(self): - - # The base class uploads a clustering task with a target - # feature. A situation where a ground truth is available - # to benchmark the clustering algorithm. - super(OpenMLClusteringTaskTest, self).test_upload_task() - - dataset_id = self._get_compatible_rand_dataset() - # Upload a clustering task without a ground truth. - task = openml.tasks.create_task( - task_type_id=self.task_type_id, - dataset_id=dataset_id, - estimation_procedure_id=self.estimation_procedure - ) - - task_id = task.publish() - TestBase._track_test_server_dumps('task', task_id) - TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], task_id)) + for i in range(100): + try: + dataset_id = self._get_compatible_rand_dataset() + # Upload a clustering task without a ground truth. + task = openml.tasks.create_task( + task_type_id=self.task_type_id, + dataset_id=dataset_id, + estimation_procedure_id=self.estimation_procedure + ) + + task_id = task.publish() + TestBase._mark_entity_for_removal('task', task_id) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + task_id)) + # success + break + except OpenMLServerException as e: + # Error code for 'task already exists' + # Should be 533 according to the docs + # (# https://www.openml.org/api_docs#!/task/post_task) + if e.code == 614: + continue + else: + raise e + else: + raise ValueError( + 'Could not create a valid task for task type ID {}'.format(self.task_type_id) + ) diff --git a/tests/test_tasks/test_task.py b/tests/test_tasks/test_task.py index 9230afbaf..b93dc9d16 100644 --- a/tests/test_tasks/test_task.py +++ b/tests/test_tasks/test_task.py @@ -56,7 +56,7 @@ def test_upload_task(self): ) task_id = task.publish() - TestBase._track_test_server_dumps('task', task_id) + TestBase._mark_entity_for_removal('task', task_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], task_id)) # success From 7c2ed4d45dadc92da38833e1d6cb882db97724ef Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Fri, 19 Jul 2019 14:42:37 +0200 Subject: [PATCH 16/17] Fixing clustering task unit test --- openml/testing.py | 2 -- tests/test_tasks/test_clustering_task.py | 3 ++- tests/test_tasks/test_task.py | 32 ++++++++++++++++-------- tests/test_utils/test_utils.py | 5 +++- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/openml/testing.py b/openml/testing.py index f15b85958..09413401c 100644 --- a/openml/testing.py +++ b/openml/testing.py @@ -31,8 +31,6 @@ class TestBase(unittest.TestCase): """ publish_tracker = {'run': [], 'data': [], 'flow': [], 'task': [], 'study': [], 'user': []} # type: dict - # legal_entities defined in openml.utils._delete_entity - {'user'} - entity_types = {'run', 'data', 'flow', 'task', 'study'} test_server = "https://test.openml.org/api/v1/xml" # amueller's read/write key that he will throw away later apikey = "610344db6388d9ba34f6db45a3cf71de" diff --git a/tests/test_tasks/test_clustering_task.py b/tests/test_tasks/test_clustering_task.py index b09eff7dc..e4654e21b 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -30,9 +30,10 @@ def test_download_task(self): self.assertEqual(task.dataset_id, 36) def test_upload_task(self): + compatible_datasets = self._get_compatible_rand_dataset() for i in range(100): try: - dataset_id = self._get_compatible_rand_dataset() + dataset_id = compatible_datasets[i % len(compatible_datasets)] # Upload a clustering task without a ground truth. task = openml.tasks.create_task( task_type_id=self.task_type_id, diff --git a/tests/test_tasks/test_task.py b/tests/test_tasks/test_task.py index b93dc9d16..3066d9ce9 100644 --- a/tests/test_tasks/test_task.py +++ b/tests/test_tasks/test_task.py @@ -1,5 +1,6 @@ import unittest -from random import randint +from typing import List +from random import randint, shuffle from openml.exceptions import OpenMLServerException from openml.testing import TestBase @@ -44,9 +45,10 @@ def test_upload_task(self): # beforehand would not be an option because a concurrent unit test could potentially # create the same task and make this unit test fail (i.e. getting a dataset and creating # a task for it is not atomic). + compatible_datasets = self._get_compatible_rand_dataset() for i in range(100): try: - dataset_id = self._get_compatible_rand_dataset() + dataset_id = compatible_datasets[i % len(compatible_datasets)] # TODO consider implementing on the diff task types. task = create_task( task_type_id=self.task_type_id, @@ -74,7 +76,7 @@ def test_upload_task(self): 'Could not create a valid task for task type ID {}'.format(self.task_type_id) ) - def _get_compatible_rand_dataset(self) -> int: + def _get_compatible_rand_dataset(self) -> List: compatible_datasets = [] active_datasets = list_datasets(status='active') @@ -82,22 +84,30 @@ def _get_compatible_rand_dataset(self) -> int: # depending on the task type, find either datasets # with only symbolic features or datasets with only # numerical features. - if self.task_type_id != 2: + if self.task_type_id == 2: + # regression task + for dataset_id, dataset_info in active_datasets.items(): + if 'NumberOfSymbolicFeatures' in dataset_info: + if dataset_info['NumberOfSymbolicFeatures'] == 0: + compatible_datasets.append(dataset_id) + elif self.task_type_id == 5: + # clustering task + compatible_datasets = list(active_datasets.keys()) + else: for dataset_id, dataset_info in active_datasets.items(): # extra checks because of: # https://github.com/openml/OpenML/issues/959 if 'NumberOfNumericFeatures' in dataset_info: if dataset_info['NumberOfNumericFeatures'] == 0: compatible_datasets.append(dataset_id) - else: - for dataset_id, dataset_info in active_datasets.items(): - if 'NumberOfSymbolicFeatures' in dataset_info: - if dataset_info['NumberOfSymbolicFeatures'] == 0: - compatible_datasets.append(dataset_id) - random_dataset_pos = randint(0, len(compatible_datasets) - 1) + # in-place shuffling + shuffle(compatible_datasets) + return compatible_datasets - return compatible_datasets[random_dataset_pos] + # random_dataset_pos = randint(0, len(compatible_datasets) - 1) + # + # return compatible_datasets[random_dataset_pos] def _get_random_feature(self, dataset_id: int) -> str: diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 04f803f86..d8ecca92a 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -48,7 +48,10 @@ def test_list_datasets_with_high_size_parameter(self): # note that in the meantime the number of datasets could have increased # due to tests that run in parallel. - self.assertGreaterEqual(len(datasets_b), len(datasets_a)) + # instead of equality of size of list, checking if a valid subset + a = set(datasets_a.keys()) + b = set(datasets_b.keys()) + self.assertTrue(b.issubset(a)) def test_list_all_for_tasks(self): required_size = 1068 # default test server reset value From 9208c4fd185205ef0f465d5985a2ed80af16c7e5 Mon Sep 17 00:00:00 2001 From: neeratyoy Date: Fri, 19 Jul 2019 16:15:29 +0200 Subject: [PATCH 17/17] Updating docs for unit test deletion --- CONTRIBUTING.md | 4 ++++ PULL_REQUEST_TEMPLATE.md | 2 ++ tests/test_flows/test_flow.py | 20 ++++++++++---------- tests/test_runs/test_run.py | 3 +++ tests/test_setups/test_setup_functions.py | 6 +++--- tests/test_study/test_study_examples.py | 2 +- tests/test_study/test_study_functions.py | 4 ++-- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b13051d67..5a77dfd58 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,6 +81,10 @@ following rules before you submit a pull request: Drafts often benefit from the inclusion of a [task list](https://github.com/blog/1375-task-lists-in-gfm-issues-pulls-comments) in the PR description. + +- Add [unit tests](https://github.com/openml/openml-python/tree/develop/tests) and [examples](https://github.com/openml/openml-python/tree/develop/examples) for any new functionality being introduced. + - If an unit test contains an upload to the test server, please ensure that it is followed by a file collection for deletion, to prevent the test server from bulking up. For example, `TestBase._mark_entity_for_removal('data', dataset.dataset_id)`, `TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name))`. + - Please ensure that the example is run on the test server by beginning with the call to `openml.config.start_using_configuration_for_example()`. - All tests pass when running `pytest`. On Unix-like systems, check with (from the toplevel source folder): diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index 4cedd1478..571ae0d1c 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -9,6 +9,8 @@ Please make sure that: * for any new function or class added, please add it to doc/api.rst * the list of classes and functions should be alphabetical * for any new functionality, consider adding a relevant example +* add unit tests for new functionalities + * collect files uploaded to test server using _mark_entity_for_removal() --> #### Reference Issue diff --git a/tests/test_flows/test_flow.py b/tests/test_flows/test_flow.py index 9f36a50f0..44b649b87 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -180,7 +180,7 @@ def test_publish_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) @@ -193,7 +193,7 @@ def test_publish_existing_flow(self, flow_exists_mock): with self.assertRaises(openml.exceptions.PyOpenMLError) as context_manager: flow.publish(raise_error_if_exists=True) - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -206,7 +206,7 @@ def test_publish_flow_with_similar_components(self): flow = self.extension.model_to_flow(clf) flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # For a flow where both components are published together, the upload @@ -225,7 +225,7 @@ def test_publish_flow_with_similar_components(self): flow1 = self.extension.model_to_flow(clf1) flow1, sentinel = self._add_sentinel_to_flow_name(flow1, None) flow1.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow1.flow_id)) @@ -237,7 +237,7 @@ def test_publish_flow_with_similar_components(self): flow2 = self.extension.model_to_flow(clf2) flow2, _ = self._add_sentinel_to_flow_name(flow2, sentinel) flow2.publish() - self._mark_entity_for_removal('flow', (flow2.flow_id, flow2.name)) + TestBase._mark_entity_for_removal('flow', (flow2.flow_id, flow2.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow2.flow_id)) # If one component was published before the other, the components in @@ -252,7 +252,7 @@ def test_publish_flow_with_similar_components(self): # Child flow has different parameter. Check for storing the flow # correctly on the server should thus not check the child's parameters! flow3.publish() - self._mark_entity_for_removal('flow', (flow3.flow_id, flow3.name)) + TestBase._mark_entity_for_removal('flow', (flow3.flow_id, flow3.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow3.flow_id)) @@ -267,7 +267,7 @@ def test_semi_legal_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -297,7 +297,7 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) @@ -365,7 +365,7 @@ def test_existing_flow_exists(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) # publish the flow flow = flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # redownload the flow @@ -426,7 +426,7 @@ def test_sklearn_to_upload_to_flow(self): flow, sentinel = self._add_sentinel_to_flow_name(flow, None) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) self.assertIsInstance(flow.flow_id, int) diff --git a/tests/test_runs/test_run.py b/tests/test_runs/test_run.py index d0c04fb9b..23ab43df0 100644 --- a/tests/test_runs/test_run.py +++ b/tests/test_runs/test_run.py @@ -13,6 +13,8 @@ import openml import openml.extensions.sklearn +import pytest + class TestRun(TestBase): # Splitting not helpful, these test's don't rely on the server and take @@ -133,6 +135,7 @@ def test_to_from_filesystem_vanilla(self): TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run_prime.run_id)) + @pytest.mark.flaky(reruns=3) def test_to_from_filesystem_search(self): model = Pipeline([ diff --git a/tests/test_setups/test_setup_functions.py b/tests/test_setups/test_setup_functions.py index 4c15830cf..16e149544 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -40,7 +40,7 @@ def test_nonexisting_setup_exists(self): flow = self.extension.model_to_flow(dectree) flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists (created as of previous statement), @@ -54,7 +54,7 @@ def _existing_setup_exists(self, classif): flow = self.extension.model_to_flow(classif) flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() - self._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], flow.flow_id)) # although the flow exists, we can be sure there are no @@ -70,7 +70,7 @@ def _existing_setup_exists(self, classif): # spoof flow id, otherwise the sentinel is ignored run.flow_id = flow.flow_id run.publish() - self._mark_entity_for_removal('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) # download the run, as it contains the right setup id run = openml.runs.get_run(run.run_id) diff --git a/tests/test_study/test_study_examples.py b/tests/test_study/test_study_examples.py index fd9cf93f1..62d1a98c8 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -51,7 +51,7 @@ def test_Figure1a(self): ) # print accuracy score print('Data set: %s; Accuracy: %0.2f' % (task.get_dataset().name, score.mean())) run.publish() # publish the experiment on OpenML (optional) - self._mark_entity_for_removal('run', run.run_id) + TestBase._mark_entity_for_removal('run', run.run_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], run.run_id)) print('URL for run: %s/run/%d' % (openml.config.server, run.run_id)) diff --git a/tests/test_study/test_study_functions.py b/tests/test_study/test_study_functions.py index 99787ca83..33ba0c452 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -77,7 +77,7 @@ def test_publish_benchmark_suite(self): task_ids=fixture_task_ids ) study_id = study.publish() - self._mark_entity_for_removal('study', study_id) + TestBase._mark_entity_for_removal('study', study_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) self.assertGreater(study_id, 0) @@ -186,7 +186,7 @@ def test_study_attach_illegal(self): run_ids=list(run_list.keys()) ) study_id = study.publish() - self._mark_entity_for_removal('study', study_id) + TestBase._mark_entity_for_removal('study', study_id) TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) study_original = openml.study.get_study(study_id)