Allow configuration aliases

[Erotemic]

What does this PR do?

This is a proof of concept that allows for aliases in config files. I've found that it works in my use case, but I don't know if it properly fits into jsonargparse the way it is currently written. I would like guidance on how this feature might be sustainably integrated.

The current change is this: If a the user defines a parse action that has multiple option strings, the normalized form of those names is now a valid config key that the user can specify in a yaml config and it maps to the dest name of the corresponding action.

Thus if the underlying ArgumentParser has an action that could be specified on the commandline as --foo=1 or --bar=1, then it can now also accept a configuration dictionary specified as foo: 1 or bar: 1. Here is an example:

    import jsonargparse
    parser = jsonargparse.ArgumentParser()
    parser.add_argument('--foo', '--bar')
    parsed = parser.parse_string('foo: 3')
    print(parsed)
    parsed = parser.parse_string('bar: 2')
    print(parsed)
    parsed = parser.parse_args(['--bar', '4'])
    print(parsed)

Currently prints:

Namespace(foo=3)
Namespace(foo=2, bar=2)
Namespace(foo='4')

In the second case I could probably fix it so it doesn't populate the alias name as well and always uses the primary key, but I'll leave that as TODO until I get feedback.

This PR has to do with #244, where I use scriptconfig to help define these aliases. Having access to aliases is important to me because it makes it much easier to transition / update variable names. I can give users a grace period where the old and new name are treated equally before writing specialized deprecation code, and finally removing the alias. Sometimes aliases are useful as permanent structures because it lets you define a CLI that works with multiple people's intuitions. Of course aliases do have drawbacks, but I hope maintainers see enough value in this feature to help me refine and merge this PR.

Before submitting

• Did you read the contributing guideline?
• Did you update the documentation? (readme and public docstrings)
• Did you write unit tests such that there is 100% coverage on related code? (required for bug fixes and new features)
• Did you verify that new and existing tests pass locally?
• Did you make sure that all changes preserve backward compatibility?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

I have not done all before submitting tasks because I need feedback before I can prepare something that is merge-ready.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[mauvilsa]

Sounds good to add support for aliases in config files. Makes it more consistent when an argument has several option strings. Thank you for working on this. I will add comments on the code for guidance.

[mauvilsa]

Additional to the other comments, note that these changes have made other unit tests fail.

[mauvilsa]

Instead of each time calling a function to transform the option_strings, the aliases can be precomputed, so that to test it can be a simple as dest in action.aliases. For example, add an aliases attribute to the base class

jsonargparse/jsonargparse/actions.py

Line 40 in 42e9c0a

class Action(LoggerProperty, ArgparseAction):

Populate the aliases attribute after this line

jsonargparse/jsonargparse/core.py

Line 139 in 42e9c0a

action.logger = self._logger # type: ignore

Here make the check simpler. Preferably don't check the same twice, i.e. if action.dest is checked, then the aliases should not include it.

[mauvilsa]

Also, I think single character options should not be aliases, e.g. o in add_argument('-o', '--output').

[Erotemic]

I like adding the aliases property. I don't see a existing mechanism in the repo for caching properties, and the minimum python is 3.6, so I can't use functools.cached_property. For now I've used hasattr and an protected underscored variant of the attribute. Not sure if you want that value actually cached, or just put into a property.

Also, now that I'm writing this I'm finding the existing tests fail because:

AttributeError: '_StoreAction' object has no attribute 'aliases'

in test_core.ParsersTests.test_precedence_of_sources.

For now I've added the attribute, but I'm still using a function (albeit a cached one). I'm not quite sure why the tests are using an action not derived from the base Action. I'll look into it, but let me know if you know why this is.

[mauvilsa]

Oh, right. Only the actions defined in jsonargparse inherit from this class. However, there are normal argparse actions and _StoreAction is one of them. Patching the argparse Action class to have this property would not be nice, but can be an option. Another option is to make it a normal attribute instead of a property and have a function (say add_aliases(action)) to add it. This function would be called for example before this line

jsonargparse/jsonargparse/core.py

Line 134 in 927ec02

if isinstance(action, ActionConfigFile) and getattr(self, '_print_config', None) is not None:

[Erotemic]

Patching of the argparse.Action class seems a bit heavy handed and prone to breakage depending on import and instance creation order.

Patching an aliases attribute in on an instance level in the place you mentioned seems more reasonable, but I'm wondering if it makes the code harder to read. For example, a reader sees this magic alias property used, but doesn't see where it comes from. On the other hand, if there is a get_aliases function (which checks for and then sets the attribute so it isn't recomputed), then it's clear where the attribute is coming from. Either of the two options will work though, so let me know if you have a preference.

[mauvilsa]

A get_aliases function that doesn't recompute does sound like the better solution.

[mauvilsa]

This feature will need more than one unit test function. Things that probably should be tested are:

• Successful parsing of a value in an alias saved as the dest key and the alias not appearing in the namespace.
• Failure to parse alias because the value does not agree with the argument type.
• Alias in a nested namespace.
• Alias with name that includes dashes -.
• Maybe more?

[Erotemic]

Agreed. Slowly working on this.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication