Skip to content

Improve user experience in config lodaing #228

Closed

Description

@steinnymir

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?

Activity

  1. rettigl commented on Nov 1, 2023

    @rettigl
    Member

    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?

  2. rettigl commented on Nov 1, 2023

    @rettigl
    Member

    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.

  3. steinnymir commented on Nov 3, 2023

    @steinnymir
    MemberAuthor

    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.

  4. rettigl commented on Nov 3, 2023

    @rettigl
    Member

    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.

  5. added this to the FLASH Beamtime milestone on Nov 4, 2023
  6. steinnymir commented on Nov 7, 2023

    @steinnymir
    MemberAuthor

    I would expect that the "save_" function overwrites only the relevant section (e.g. energy) and leaves the rest as is.

    I could not reproduce this fully. It does re-write the .yaml file but the info seems to remain the same, just shuffled around.

  7. changed the title [-]config loading unintended files[/-] [+]Improve user experience in config lodaing[/+] on Nov 9, 2023
  8. removed this from the FLASH Beamtime milestone on Nov 9, 2023
  9. locked and limited conversation to collaborators on Feb 1, 2024
  10. converted this issue into a discussion #338 on Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions