You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently the loading of a configuration when creating the SedProcessor is extremely greedy. It loads a bunch of settings files (which should be the defaults I guess) in a convoluted way, and I have not yet figured out how to overrule this.
I would have expected a single argument config in SedProcessor.init which you have to interact with.
It makes sense that when you pass it an incomplete config file (probably the one in your working folder, but can be also somewhere else) it tries to complete it with the defaults.
Is it necessary to always pick what you have in the working folder? this one is often not what you expect, and can lead to funny behaviours (i.e. on maxwell through ssh, it happens to be your home dir, not where the notebook is...)
Maybe we can discuss a more user-friendly approach?
I thought this is what we discussed. It anyways reports what it loads, so is transparent in this regard. You can always disable any single config type by passing an empty dictionary (see examples in the processor tests). Config files that are not present are also not loaded, and these names are specific enough to not be there by chance, so a user should be aware if he placed a corresponding file at the respective place, no?
Is it necessary to always pick what you have in the working folder? this one is often not what you expect, and can lead to funny behaviours (i.e. on maxwell through ssh, it happens to be your home dir, not where the notebook is...)
This mechanism (folder_config) was introduced to allow easy passing of calibration parameters between notebooks. I think it is a good approach, used by the save_... functions in the processor.
I just felt this was quite convoluted and not intuitive, even though I know we agreed on this previously.
However, the "save_" functions did something I did not expect them to, which was to overwrite anything in the target file with default settings unrelated to those currently loaded, which broke the settings file for that run.
I would expect that the "save_" function overwrites only the relevant section (e.g. energy) and leaves the rest as is.
Saving of all settings, including calibrations could be done by a separate function in SedProcessor. This could then make a copy of the config dict completely overwriting the file.
However, the "save_" functions did something I did not expect them to, which was to overwrite anything in the target file with default settings unrelated to those currently loaded, which broke the settings file for that run.
It should not do so, can you provide an example of this? It should update the keys given by the respective "to_save" branch, and save the config again.
Activity
rettigl commentedon Nov 1, 2023
I thought this is what we discussed. It anyways reports what it loads, so is transparent in this regard. You can always disable any single config type by passing an empty dictionary (see examples in the processor tests). Config files that are not present are also not loaded, and these names are specific enough to not be there by chance, so a user should be aware if he placed a corresponding file at the respective place, no?
rettigl commentedon Nov 1, 2023
This mechanism (folder_config) was introduced to allow easy passing of calibration parameters between notebooks. I think it is a good approach, used by the save_... functions in the processor.
steinnymir commentedon Nov 3, 2023
I just felt this was quite convoluted and not intuitive, even though I know we agreed on this previously.
However, the "save_" functions did something I did not expect them to, which was to overwrite anything in the target file with default settings unrelated to those currently loaded, which broke the settings file for that run.
I would expect that the "save_" function overwrites only the relevant section (e.g. energy) and leaves the rest as is.
Saving of all settings, including calibrations could be done by a separate function in SedProcessor. This could then make a copy of the config dict completely overwriting the file.
rettigl commentedon Nov 3, 2023
It should not do so, can you provide an example of this? It should update the keys given by the respective "to_save" branch, and save the config again.
steinnymir commentedon Nov 7, 2023
I could not reproduce this fully. It does re-write the .yaml file but the info seems to remain the same, just shuffled around.
[-]config loading unintended files[/-][+]Improve user experience in config lodaing[/+]