-
-
Notifications
You must be signed in to change notification settings - Fork 53
What's the best way to do backwards compatibility for existing configs? #479
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.
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
Comments
My ideal solution would be to keep the feature of having a sync between the source code and the CLIs, and not make up something new specific for CLIs. That is, it would be nice to adopt some deprecated standard, so that deprecated decorators are added to the code, and based on that jsonargparse runs additional logic. For example if a parameter In projects that I have worked on, there are three approaches we have taken, though they haven't used specific features of jsonargparse.
What to recommend, I am not sure. But we can brainstorm further. |
2) is what I would prefer. But how does one integrate it into the parser? Is there an extension point in |
Currently there isn't such a feature. But note that it isn't just |
Actually, there is a feature that could be used, though it wasn't intended for this: custom-loaders. You can implement a custom loader. The default yaml_load has some logic which avoids weird behaviors of All parsed values go through the loader. So depending on what is loaded, the function would need to decide if a transformation is required. |
We'll try this and report back. Only handling configs is acceptable at the moment since it's how we recommend that people interact with the program. |
With #543 now it is possible to get the default yaml loader. |
Thank you Mauricio. I won't be able to check this with my original script. Feel free to close the issue if the current implementation is good enough. |
I didn't expose the loader particularly for this issue. But if you aren't going to look into this further, then yes I guess we can close the issue. |
@mauvilsa I'm trying this feature now (sorry! it's been a while). Can you re-open this? Here's my updated code to use the loader feature class Foo:
def __init__(self, b=2):
...
def fn(foo: Foo = Foo()):
...
from jsonargparse import ArgumentParser, ActionConfigFile
from jsonargparse import get_loader, set_loader
def apply_backwards_compatibility_transformations(config):
print("Transforming", config)
if not isinstance(config, dict):
# it is loading something else
return config
config = config.copy()
config["foo"]["init_args"].setdefault("b", config["foo"]["init_args"].pop("a", None))
return config
def enable_backwards_compatibility(parser: ArgumentParser) -> None:
loader = get_loader(mode=parser.parser_mode)
def custom(config_input: str):
config = loader(config_input)
return apply_backwards_compatibility_transformations(config)
set_loader("backwards_compatibility_loader", custom)
parser.parser_mode = "backwards_compatibility_loader"
parser = ArgumentParser(parser_mode="yaml")
enable_backwards_compatibility(parser)
parser.add_argument("-c", "--config", action=ActionConfigFile)
parser.add_function_arguments(fn)
args = parser.parse_args()
print(args) # before.yaml
foo:
class_path: __main__.Foo
init_args:
a: 2 Now, However, if I override an argument: Then, my code doesnt work because What if jsonargparse provided an extension point that allowed this sort of transformation on the final args? After all the inputs have been loaded but before any config validation or class instantiation. Thanks! |
This is not simple. You could subclass A solution could be a decorator to mark parameter deprecations, even though there isn't a standard for this in python yet. Is there any parsing library that does something like this, be it python or other language? Would be good to know for inspiration. Still, not the most simple topic, so I would still go for something like #479 (comment) option 3. |
I wouldn't want option 3 for the follwing reasons:
class Bar:
def __init__(self, a=2):
...
def fn(bar: Bar = Bar()):
... You could think of arbitrary transformations to the config that would be very difficult to support just by using Overall having a single global opportunity to transform the config would be ideal. |
I have thought about this carefully and I think it is not a good solution. There are several reasons:
I have some ideas I can propose something which wouldn't require custom |
I have created a package to illustrate my idea. You can do from deprecated_parameters import ParameterRename, deprecated_parameters
class Foo:
@deprecated_parameters(
ParameterRename(old_name="a", new_name="b"),
)
def __init__(self, b=2):
self.b = b
foo = Foo(a=3)
print(foo.b) During runtime, by default the decorator takes care of changing the argument from Anyway, the point is that |
If I have a CLI implementation (
before.py
) with aFoo.a
argumentpython before.py --print_config > a.yaml
Where
a.yaml
is:And then create an
after.py
file whose only difference is renaminga
tob
What's the best or recommended way to support loading
a.yaml
withafter.py
, wherea
is remapped tob
? It currently fails as expected with:Context
This is currently blocking a rename in Lightning-AI/litgpt#1156
The text was updated successfully, but these errors were encountered: