Unexpected behavior: unconditional initialization with "default_settings.txt"

[schmittner]

Problem description by example: Let new_settings.txt define a new scenario with two groups but only sets a default speed for both groups: Group.speed = 0.5, 1.5. Since there are no explicit settings for Group1.speed or Group2.speed once would expect both groups to be set to the default speed when calling ./one.sh new_settings.txt.
This is not the case: Since default_settings.txt defines explicitly Group2.speed = 2.7, 13.9, this (specialized) setting will be applied to group 2 of new_settings.txt because the Properties object is defaulted with default_settings.txt if it exists.

This is unexpected behavior and should be removed. Note that if default_settings.txtis deleted or directly changed, this is not a problem. There are no warning messages whether default_settings.txt is used or not.

This patch does not change the default behavior of starting ONE with default_settings.txt if called without any arguments.

[tk721]

This indeed trips up many users, but the behavior is by design. The idea behind always loading default_settings.txt is that the basic/common behavior can go into that file, and the specialized behavior into the settings file passed as a parameter. Likewise the more specific settings overriding the more general setting allows for a basic/common behavior with specialization. Unfortunately these two behaviors -- reasonable by themselves -- combine to unintuitive behavior.

Personally I'm fine with changing the default_settings.txt behavior since I've seen it cause problems enough times that I now advice people to just delete it. Perhaps it would make sense to allow multiple settings files to be passed explicitly to get the same behavior when desired.

The problem, however, is that this change will break all existing settings that rely on the default_settings.txt being always loaded.

[schmittner]

Well, existing settings will not break, but they will have to be run by including default settings explicitly (this already works):

./one.sh default_settings.txt old_setting.txt

[tk721]

Right, then this seems like a good tradeoff to me. Maybe print a warning for a while that the default settings are not getting loaded anymore.

[schmittner]

Added the warning when default_settings.txt is not one of explicitly loaded setting files.

[akeranen]

Fair enough. This has never been a problem for me, but I can see how it's not really the most intuitive design. I think (temporary?) warning about this chage when settings are not found could be in order.

[schmittner]

I'm not exactly sure what you mean by the (additional?) warning. I added a warning that is printed if calling

./one.sh new_settings.txt

but not for calls such as

# implicitly using default_settings.txt
./one.sh
# explicitly using default_settings.txt
./one.sh default_settings.txt new_settings.txt

If any file that is to be loaded is missing, Settings will fire a SettingsError (which already was the case).

[akeranen]

Seems that this variable is never used

[akeranen]

What I meant was that if a variable that is required for some module is not defined in the given settings files, it could be useful to add to the thrown SettingsError description message (in Settings.getSetting method) a hint that the behavior has changed (e.g., saying "note that default_settings.txt is no longer automatically used").

[schmittner]

I missed to include the actual warning message in the patch (my bad). Fixed now.

[schmittner]

@akeranen Is there anything else to be done?

[julianofischer]

IMO this behavior is indeed confusing.
Maybe the default_settings.txt should be excluded (renamed).
I think there are users really lost because they dont know that default_settings is "always" loaded.

The key is: if the user know how default_settings.txt work, he can use it (basically renaming or creating a new one - unintentional pun). If dont, it is better do not "force" the use.