From bc19dfe5c595b3644734e39fb07ec37574a05575 Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 8 Dec 2021 11:45:12 -0500 Subject: [PATCH 1/3] Remove any checks for the `log` config field. --- skll/config/__init__.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/skll/config/__init__.py b/skll/config/__init__.py index e30a0a11..31319fb3 100644 --- a/skll/config/__init__.py +++ b/skll/config/__init__.py @@ -68,7 +68,6 @@ def __init__(self): 'id_col': 'id', 'ids_to_floats': 'False', 'label_col': 'y', - 'log': '', 'logs': '', 'learning_curve_cv_folds_list': '[]', 'learning_curve_train_sizes': '[]', @@ -113,7 +112,6 @@ def __init__(self): 'id_col': 'Input', 'ids_to_floats': 'Input', 'label_col': 'Input', - 'log': 'Output', 'logs': 'Output', 'learning_curve_cv_folds_list': 'Input', 'learning_curve_train_sizes': 'Input', @@ -451,18 +449,6 @@ def parse_config_file(config_path, log_level=logging.INFO): # noqa: C901 # next, get the log path before anything else since we need to # save all logging messages to a log file in addition to displaying # them on the console - - # if the user specified "log" instead of "logs", we need - # to show a warning and save its value in "logs" instead - log_value = config.get("Output", "log") - show_log_warning = False - if log_value: - # since we do not have a logger yet, we will show the warning later - show_log_warning = True - config.set('Output', 'logs', log_value) - config.remove_option('Output', 'log') - - # now get the value from the 'logs' field log_value = config.get("Output", "logs") try: @@ -483,13 +469,6 @@ def parse_config_file(config_path, log_level=logging.INFO): # noqa: C901 filepath=main_log_file, log_level=log_level) - # now show the 'log' warning, if any, since we have the logger set up - if show_log_warning: - logger.warning("The 'log' option in the [Output] section is deprecated " - "and will be removed in the next version. Use 'logs' " - "instead.") - show_log_warning = False - if config.has_option("General", "task"): task = config.get("General", "task") else: From 7cd0d84432ce1de4d1d855667b6d0e60e23df5a0 Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 8 Dec 2021 11:45:43 -0500 Subject: [PATCH 2/3] Update the test - The test that uses `log` in the config file should now raise a KeyError instead of a warning. --- tests/test_input.py | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/tests/test_input.py b/tests/test_input.py index c1850380..9e13b9c2 100644 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -2030,9 +2030,10 @@ def test_config_parsing_set_save_votes_value(): eq_(save_votes, True) +@raises(KeyError) def test_config_parsing_use_log_instead_of_logs(): """ - Check that using 'log' instead of 'logs' raises a warning + Check that using 'log' instead of 'logs' raises an error """ values_to_fill_dict = { @@ -2054,24 +2055,4 @@ def test_config_parsing_use_log_instead_of_logs(): values_to_fill_dict, 'use_log_vs_logs') - (experiment_name, task, sampler, fixed_sampler_parameters, - feature_hasher, hasher_features, id_col, label_col, train_set_name, - test_set_name, suffix, featuresets, do_shuffle, model_path, - do_grid_search, grid_objectives, probability, pipeline, results_path, - pos_label_str, feature_scaling, min_feature_count, folds_file, - grid_search_jobs, grid_search_folds, cv_folds, save_cv_folds, - save_cv_models, use_folds_file_for_grid_search, do_stratified_folds, - fixed_parameter_list, param_grid_list, featureset_names, learners, - prediction_dir, log_path, train_path, test_path, ids_to_floats, - class_map, custom_learner_path, custom_metric_path, learning_curve_cv_folds_list, - learning_curve_train_sizes, output_metrics, save_votes) = parse_config_file(config_path) - - log_file_on_disk = join(output_dir, "config_parsing_log_vs_logs.log") - with open(log_file_on_disk) as f: - warning_pattern = re.compile( - r"The 'log' option in the \[Output\] section is deprecated" - ) - matches = re.findall(warning_pattern, f.read()) - eq_(len(matches), 1) - - eq_(log_path, output_dir) + parse_config_file(config_path) From 48c48c70a60b09f75049626e996b33c2e3f98ad4 Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 8 Dec 2021 12:20:25 -0500 Subject: [PATCH 3/3] Update all tests to use `logs` instead of `log`. --- tests/test_classification.py | 8 ++++---- tests/test_output.py | 6 +++--- tests/test_regression.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_classification.py b/tests/test_classification.py index 208895b2..f338065c 100644 --- a/tests/test_classification.py +++ b/tests/test_classification.py @@ -208,7 +208,7 @@ def check_label_index_order_with_pos_label_str(use_api=True): 'train_file': train_file, 'grid_search': 'false', 'pos_label_str': str(pos_label_str), - 'log': output_dir, + 'logs': output_dir, 'models': output_dir} config_path = fill_in_config_options(config_template_path, @@ -318,7 +318,7 @@ def check_binary_predictions_for_pos_label_str(label_list, 'train_file': train_file, 'grid_search': 'false', 'pos_label_str': str(pos_label_str), - 'log': output_dir, + 'logs': output_dir, 'models': output_dir, 'probability': 'true' if probability else 'false'} @@ -1447,7 +1447,7 @@ def check_metric_values_for_classification(metric_name, values_to_fill_dict = {'experiment_name': experiment_name, 'train_file': train_file, 'test_file': test_file, - 'log': output_dir, + 'logs': output_dir, 'models': output_dir, 'results': output_dir, 'predictions': output_dir, @@ -1661,7 +1661,7 @@ def check_metrics_and_objectives_overlap(task, metrics, objectives): 'task': task, 'train_file': train_file, 'learners': "['LogisticRegression']", - 'log': output_dir, + 'logs': output_dir, 'probability': 'false', 'results': output_dir, 'predictions': output_dir, diff --git a/tests/test_output.py b/tests/test_output.py index d971246a..552c30e1 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -349,7 +349,7 @@ def check_xval_fancy_results_file(do_grid_search, 'num_cv_folds': '6', 'grid_search_folds': '4', 'learners': "['LogisticRegression']", - 'log': output_dir, + 'logs': output_dir, 'predictions': output_dir, 'results': output_dir} @@ -458,7 +458,7 @@ def time_field(x): 'objectives': "['f1_score_micro']", 'featureset_names': "['f0']", 'learners': json.dumps(learners), - 'log': output_dir, + 'logs': output_dir, 'results': output_dir} if task == 'train': values_to_fill_dict['models'] = output_dir @@ -611,7 +611,7 @@ def test_multiple_featuresets_and_featurehasher_throws_warning(): "[['test_input_3examples_1', 'test_input_3examples_2']]", "featureset_names": "['feature_hasher']", 'suffix': '.jsonlines', - 'log': output_dir, + 'logs': output_dir, 'models': output_dir, 'feature_hasher': "true", "hasher_features": "4" diff --git a/tests/test_regression.py b/tests/test_regression.py index 3b73a08d..35ac07ca 100644 --- a/tests/test_regression.py +++ b/tests/test_regression.py @@ -448,7 +448,7 @@ def test_int_labels(): config.set("Input", "train_file", join(other_dir, 'test_int_labels_cv.jsonlines')) config.set("Output", "results", output_dir) - config.set("Output", "log", output_dir) + config.set("Output", "logs", output_dir) config.set("Output", "predictions", output_dir) with open(config_path, 'w') as new_config_file: