Skip to content

Deprecate log field in configuration files. #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
desilinguist merged 3 commits into from
Dec 8, 2021
Merged

Deprecate log field in configuration files. #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc19dfe
Remove any checks for the `log` config field.
desilinguist Dec 8, 2021
7cd0d84
Update the test
desilinguist Dec 8, 2021
48c48c7
Update all tests to use `logs` instead of `log`.
desilinguist Dec 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions skll/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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': '[]',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions tests/test_classification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 3 additions & 22 deletions tests/test_input.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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)
6 changes: 3 additions & 3 deletions tests/test_output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_regression.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down