Open jhaux opened 4 years ago
Great stuff! A couple of suggestions: Let's try to ensure we use the same behavior everywhere. For example https://github.com/pesser/edflow/blob/36a24eac501f2b4b4e2475ec88e51e9017c8c13f/edflow/eval/pipeline.py#L748
any other places? I think it would be best to move
load_config
(edflow/edflow l. 24) toutils
and rely on that function in all other places. I would also really like either an option or a dedicated command that just prints a final config resulting from a combination of-b <configs..>
and--key/path value
command line arguments --- one can then easily check that the config looks as intended and pipe the resulting config to a file to put it under version control and reuse it.
I will put up a seperate pull request for unified behavior. This should not be too hard but needs proper documentation, such that we can make it easy to add new entry points, which follow the same principles.
The fully updated and final config is displayed richt before iteration starts. I think this should be the point where it is printed, as afterwards, we want it to be fixed.
Agreed! Let's implement the unified entry point handling. Nevertheless I think, we should enhance this PR to solve #255 and not start a new PR, as I am certain, that we would simply reinvent some features that this PR introduces (especially update and merge). What do you think?
ping. Can we get this done soon so that we can make a new release version?
This solves #216 partly. The stacking behavior is as described in the issue: Configs are merged using
edflow.util.merge
which in turn usesedflow.util.update
in the background. Both of these functions are now properly documented, but there is no proper high level documentation.