PluginRequirements: Fix optional and no default value behaviour

[gcmoreira]

IMO the current implementation of an optional argument requirement with no default value is not working as expected.
For instance, if I use the following and no --output is used, I would expect the self.config doesn't contain the output key.

            requirements.StringRequirement(
                name="output",
                description="Output filename",
                optional=True,
            ),

So when I do this:

output_name = self.config.get("output", default_output_name)

In HierarchicalDict::getitem() it will raise a KeyError exception and the getter will assign the default value.

However, that is not working. The keys are created anyway, regardless of the optional=True and the absence of a default value.
That means, in this case, calling self.config.get("output", "foo") from a plugin still returns None.

[gcmoreira]

One possible fix for this is only to set the default value if optional is False here. Thoughts?

[ikelos]

I'm not keen on changing this behaviour, because there may be existing plugins that expect all keys to always exist, which will then throw an exception when they don't. The configuration value is set by the plugin, so moving the default value from a self.config.get to a default = seems like a straight forward work around?

It's basically two ways of doing the same thing. The current established way of doing this is that if a configuration option is defined, you can guarantee the key will be present. It should default to None, meaning that self.config.get('output') or foo should have a similar effect. Unless there's a very compelling reason why this way of doing things is wrong rather than just different, I don't want to introduce a change to our existing system.

[ikelos]

Also, please see the discussion on #324 and the solution #354 which may be relevant.