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/doc/progress.rst b/doc/progress.rst index c6733dbc8..f2e0bc90d 100644 --- a/doc/progress.rst +++ b/doc/progress.rst @@ -8,14 +8,17 @@ Changelog 0.10.0 ~~~~~~ -* ADD #722: Automatic reinstantiation of flow in `run_model_on_task`. Clearer errors if that's not possible. +* 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 #608: Fixing dataset_id referenced before assignment error in get_run function. -* ADD #715: `list_evaluations` now has an option to sort evaluations by score (value). +* FIX #447: All files created by unit tests are deleted after the completion of all unit tests. * 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. -* FIX #447: All files created by unit tests are deleted after the completion of all unit tests. +* 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. * MAINT #726: Update examples to remove deprecation warnings from scikit-learn 0.9.0 diff --git a/openml/testing.py b/openml/testing.py index 9b6649c97..09413401c 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,6 +29,18 @@ class TestBase(unittest.TestCase): Currently hard-codes a read-write key. Hopefully soon allows using a test server, not the production server. """ + publish_tracker = {'run': [], 'data': [], 'flow': [], 'task': [], + 'study': [], 'user': []} # 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. @@ -46,6 +59,7 @@ 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 @@ -71,12 +85,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" - - openml.config.server = self.test_server + openml.config.server = TestBase.test_server openml.config.avoid_duplicate_runs = False openml.config.cache_directory = self.workdir @@ -87,7 +98,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 @@ -104,9 +115,43 @@ def tearDown(self): openml.config.server = self.production_server openml.config.connection_n_retries = self.connection_n_retries + @classmethod + 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'. + 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.publish_tracker: + TestBase.publish_tracker[entity_type] = [entity_id] + else: + TestBase.publish_tracker[entity_type].append(entity_id) + + @classmethod + def _delete_entity_from_tracker(self, entity_type, entity): + """ 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): - """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. @@ -125,7 +170,6 @@ def _cleanup_fixture(self): else: static_cache_dir = os.path.join(static_cache_dir, '../') directory = os.path.join(static_cache_dir, 'tests/files/') - # directory = "{}/tests/files/".format(static_cache_dir) files = os.walk(directory) old_file_list = [] for root, _, filenames in files: @@ -135,6 +179,10 @@ 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: @@ -142,10 +190,40 @@ 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 + # + openml.config.server = TestBase.test_server + openml.config.apikey = TestBase.apikey + + # 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() + + # 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: + 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 + for entity_type in entity_types: + for i, entity in enumerate(tracker[entity_type]): + try: + openml.utils._delete_entity(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: + 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: # 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 3f68b467d..80d7333a0 100644 --- a/tests/test_datasets/test_dataset_functions.py +++ b/tests/test_datasets/test_dataset_functions.py @@ -478,6 +478,9 @@ def test_publish_dataset(self): data_file=file_path, ) dataset.publish() + 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) def test__retrieve_class_labels(self): @@ -498,6 +501,9 @@ def test_upload_dataset_with_url(self): url="https://www.openml.org/data/download/61/dataset_61_iris.arff", ) dataset.publish() + 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) def test_data_status(self): @@ -507,6 +513,9 @@ def test_data_status(self): version=1, url="https://www.openml.org/data/download/61/dataset_61_iris.arff") dataset.publish() + 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 # admin key for test server (only adminds can activate datasets. @@ -620,6 +629,9 @@ def test_create_dataset_numpy(self): ) upload_did = dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), @@ -682,6 +694,9 @@ def test_create_dataset_list(self): ) upload_did = dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -725,6 +740,9 @@ def test_create_dataset_sparse(self): ) upload_did = xor_dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -762,6 +780,9 @@ def test_create_dataset_sparse(self): ) upload_did = xor_dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), xor_dataset._dataset, @@ -885,6 +906,9 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -919,6 +943,9 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() + TestBase._mark_entity_for_removal('data', upload_did) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + upload_did)) self.assertEqual( _get_online_dataset_arff(upload_did), dataset._dataset, @@ -955,6 +982,9 @@ def test_create_dataset_pandas(self): paper_url=paper_url ) upload_did = dataset.publish() + 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) self.assertEqual( downloaded_data, @@ -1123,6 +1153,9 @@ def test___publish_fetch_ignore_attribute(self): # publish dataset upload_did = dataset.publish() + TestBase._mark_entity_for_removal('data', 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() @@ -1253,6 +1286,9 @@ def test_create_dataset_row_id_attribute_inference(self): ) self.assertEqual(dataset.row_id_attribute, output_row_id) upload_did = dataset.publish() + 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)) 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 aef064ad5..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,6 +1126,8 @@ 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._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 7b8c66cab..44b649b87 100644 --- a/tests/test_flows/test_flow.py +++ b/tests/test_flows/test_flow.py @@ -41,6 +41,9 @@ def setUp(self): super().setUp() self.extension = openml.extensions.sklearn.SklearnExtension() + def tearDown(self): + super().tearDown() + def test_get_flow(self): # We need to use the production server here because 4024 is not the # test server @@ -177,6 +180,9 @@ def test_publish_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() + 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) @mock.patch('openml.flows.functions.flow_exists') @@ -187,6 +193,9 @@ 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) + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) self.assertTrue('OpenMLFlow already exists' in context_manager.exception.message) @@ -197,6 +206,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() + 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 # date should be equal self.assertEqual( @@ -213,6 +225,9 @@ 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() + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + 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) @@ -222,6 +237,9 @@ 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() + 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 # the flow should have different upload dates self.assertNotEqual(flow2.upload_date, @@ -234,6 +252,9 @@ 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() + TestBase._mark_entity_for_removal('flow', (flow3.flow_id, flow3.name)) + 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! @@ -246,6 +267,9 @@ def test_semi_legal_flow(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) flow.publish() + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + 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') @@ -260,6 +284,8 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): get_flow_mock.return_value = flow flow.publish() + # 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) @@ -271,6 +297,9 @@ def test_publish_error(self, api_call_mock, flow_exists_mock, get_flow_mock): with self.assertRaises(ValueError) as context_manager: flow.publish() + TestBase._mark_entity_for_removal('flow', (flow.flow_id, flow.name)) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], + flow.flow_id)) fixture = ( "Flow was not stored correctly on the server. " @@ -336,6 +365,9 @@ def test_existing_flow_exists(self): flow, _ = self._add_sentinel_to_flow_name(flow, None) # publish the flow flow = flow.publish() + 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 flow = openml.flows.get_flow(flow.flow_id) @@ -394,6 +426,9 @@ def test_sklearn_to_upload_to_flow(self): flow, sentinel = self._add_sentinel_to_flow_name(flow, None) flow.publish() + 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) # 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 f0001ac96..02d4b2a7d 100644 --- a/tests/test_flows/test_flow_functions.py +++ b/tests/test_flows/test_flow_functions.py @@ -4,6 +4,7 @@ from distutils.version import LooseVersion import sklearn +from sklearn import ensemble import pandas as pd import openml @@ -14,6 +15,12 @@ class TestFlowFunctions(TestBase): _multiprocess_can_split_ = True + def setUp(self): + super(TestFlowFunctions, self).setUp() + + def tearDown(self): + super(TestFlowFunctions, self).tearDown() + def _check_flow(self, flow): self.assertEqual(type(flow), dict) self.assertEqual(len(flow), 6) @@ -242,7 +249,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 @@ -251,17 +257,20 @@ 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._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) 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._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) 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 bba14b324..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 @@ -129,7 +131,11 @@ 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._mark_entity_for_removal('run', run_prime.run_id) + 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([ @@ -162,6 +168,9 @@ 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._mark_entity_for_removal('run', 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): @@ -226,6 +235,9 @@ 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._mark_entity_for_removal('run', 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 5e0f48264..bd123cd37 100644 --- a/tests/test_runs/test_run_functions.py +++ b/tests/test_runs/test_run_functions.py @@ -184,6 +184,8 @@ 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._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) @@ -196,6 +198,8 @@ def _remove_random_state(flow): avoid_duplicate_runs=openml.config.avoid_duplicate_runs, ) run_ = run.publish() + 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) @@ -687,6 +691,8 @@ def test_initialize_cv_from_run(self): seed=1, ) run_ = run.publish() + 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) modelR = openml.runs.initialize_model_from_run(run_id=run.run_id) @@ -802,6 +808,8 @@ def test_initialize_model_from_run(self): avoid_duplicate_runs=False, ) run_ = run.publish() + 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) modelR = openml.runs.initialize_model_from_run(run_id=run.run_id) @@ -853,6 +861,8 @@ def test_get_run_trace(self): num_iterations * num_folds, ) run = run.publish() + 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 except openml.exceptions.OpenMLRunsExistError as e: @@ -897,6 +907,8 @@ def test__run_exists(self): upload_flow=True ) run.publish() + 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. pass @@ -957,6 +969,8 @@ 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._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): # Check the case where the user adds an illegal flow id to an existing @@ -966,6 +980,8 @@ 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._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 pass @@ -991,6 +1007,8 @@ 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._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 pass @@ -1263,6 +1281,8 @@ 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._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) task = openml.tasks.get_task(119) # diabetes @@ -1274,3 +1294,5 @@ def test_run_flow_on_task_downloaded_flow(self): ) run.publish() + 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 a8f7de4d4..16e149544 100644 --- a/tests/test_setups/test_setup_functions.py +++ b/tests/test_setups/test_setup_functions.py @@ -40,6 +40,8 @@ def test_nonexisting_setup_exists(self): flow = self.extension.model_to_flow(dectree) flow.name = 'TEST%s%s' % (sentinel, flow.name) flow.publish() + 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), # we can be sure there are no setups (yet) as it was just created @@ -52,6 +54,8 @@ def _existing_setup_exists(self, classif): flow = self.extension.model_to_flow(classif) flow.name = 'TEST%s%s' % (get_sentinel(), flow.name) flow.publish() + 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 # setups (yet) as it hasn't been ran @@ -66,6 +70,8 @@ def _existing_setup_exists(self, classif): # spoof flow id, otherwise the sentinel is ignored run.flow_id = flow.flow_id run.publish() + 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 abee2d72a..62d1a98c8 100644 --- a/tests/test_study/test_study_examples.py +++ b/tests/test_study/test_study_examples.py @@ -51,4 +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) + 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 c87dd8e15..33ba0c452 100644 --- a/tests/test_study/test_study_functions.py +++ b/tests/test_study/test_study_functions.py @@ -77,6 +77,9 @@ def test_publish_benchmark_suite(self): task_ids=fixture_task_ids ) study_id = study.publish() + TestBase._mark_entity_for_removal('study', study_id) + TestBase.logger.info("collected from {}: {}".format(__file__.split('/')[-1], study_id)) + self.assertGreater(study_id, 0) # verify main meta data @@ -132,6 +135,8 @@ def test_publish_study(self): run_ids=list(run_list.keys()) ) 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) @@ -181,6 +186,8 @@ def test_study_attach_illegal(self): run_ids=list(run_list.keys()) ) study_id = study.publish() + 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) 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 21e03052f..e4654e21b 100644 --- a/tests/test_tasks/test_clustering_task.py +++ b/tests/test_tasks/test_clustering_task.py @@ -1,5 +1,7 @@ import openml +from openml.testing import TestBase from .test_task import OpenMLTaskTest +from openml.exceptions import OpenMLServerException class OpenMLClusteringTaskTest(OpenMLTaskTest): @@ -28,19 +30,32 @@ 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() - openml.utils._delete_entity('task', task_id) + compatible_datasets = self._get_compatible_rand_dataset() + for i in range(100): + try: + 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, + 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 fe7fa5f0e..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 @@ -11,9 +12,6 @@ create_task, get_task ) -from openml.utils import ( - _delete_entity, -) class OpenMLTaskTest(TestBase): @@ -47,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, @@ -59,6 +58,9 @@ def test_upload_task(self): ) 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: @@ -74,9 +76,7 @@ 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: + def _get_compatible_rand_dataset(self) -> List: compatible_datasets = [] active_datasets = list_datasets(status='active') @@ -84,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