search

audeering / audobject

Generic Python interface for serializing objects to YAML
https://audeering.github.io/audobject/
Other
1 stars 0 forks source link

from_dict(), from_yaml(), from_yaml_s(): deprecate **kwargs for override_args #27

Closed frankenjoe closed 3 years ago

frankenjoe commented 3 years ago

Closes #26

As reported in #26 it is not possible to override arguments named root or d with from_yaml() and from_yaml_s(). This is because from_dict() (which is called by the other functions) has arguments with those names, i.e. we have a clash if we also try to pass them also as keyword arguments. As a solution, **kwargs is deprecated and instead additional keyword arguments can be passed as a dictionary.

The other solution proposed in #26 would be to rename the arguments of from_dict() to something that is not likely chosen by the user. However, the solution proposed here seems to be cleaner. The name override_args also expresses better its purpose than **kargs.

A test is added to verify we can now override an argument with name root.

image

codecov[bot] commented 3 years ago

Codecov Report

Merging #27 (e35cf04) into master (3d93b62) will not change coverage. The diff coverage is 100.0%.

Impacted Files Coverage Δ
audobject/core/api.py 100.0% <100.0%> (ø)
audobject/core/utils.py 100.0% <100.0%> (ø)
frankenjoe commented 3 years ago

Note that it is still not possible to override arguments path_or_stream and include_version with to_yaml(), as these are arguments of to_yaml(). Of course, we could also change kwargs to a dictionary in to_yaml(), but that would break a lot of code. But at least here it is more obvious for the user that this is not possible.

I decided to also deprecate the use of **kwargs** with to_yaml() and to_yaml_s().

hagenw commented 3 years ago

As **kwargs is deprecated I think it should not appear in the documentation, e.g.

image

frankenjoe commented 3 years ago

As **kwargs is deprecated I think it should not appear in the documentation, e.g.

But if we remove it from the function signature, we are no longer backward compatible.

hagenw commented 3 years ago

As **kwargs is deprecated I think it should not appear in the documentation, e.g.

But if we remove it from the function signature, we are no longer backward compatible.

Yes, you are right. So I guess we just stick with it until we remove it completely then.