Ulm-IQO / qudi-core

A framework for modular measurement applications.
GNU General Public License v3.0
25 stars 19 forks source link

Configuration overhaul #25

Closed Neverhorst closed 1 year ago

Neverhorst commented 2 years ago

Description

The entire structure, the typing and default values for the qudi config are now defined in a more standardized way as JSON Schema (Draft-7) in qudi.core.config.schema.
Since all default values are implemented in this schema, got also rid of qudi.core.default.cfg.

The qudi.core.config.Configuration object will now only perform schema validation as well as provide a convenient mapping and functional interface to a qudi config instead of explicitly implementing each field as Python property,

ConfigOption meta attributes are now set under their own subsection options in the module config section.
WARNING: This comes with a breaking change since users have to change their module configurations from e.g.

my_module_name:
    module.Class: 'qudi.hardware.my_module.MyModule'
    first_option: 42
    second_option: 'hello world'

to

my_module_name:
    module.Class: 'qudi.hardware.my_module.MyModule'
    options:
        first_option: 42
        second_option: 'hello world'

Motivation and Context

Defining the qudi configuration via JSON Schema is a way more flexible, transparent and standardized approach than changing the Configuration class any time a change is introduced on the config structure.

Also moving the ConfigOption fields into their own options section prevents namespace clashes with (future) inherent module config options.

How Has This Been Tested?

Types of changes

Checklist:

timoML commented 2 years ago

I'm getting an error on falling back to the default.cfg that ends up with an completly empty manager being shown. Or is this supposed to be the error while parsing the errornous cfg? Then the error message is a little misleading and I would expect to see some dummy modules. I guess that's what the old default.cfg was doing.

  Traceback (most recent call last):
    File "C:\Users\t\OneDrive\_Promotion\Software\qudi_newrepo\qudi-core\src\qudi\core\application.py", line 118, in __init__
      self.configuration.load_config(config_file, set_default=True)
    File "C:\Users\t\OneDrive\_Promotion\Software\qudi_newrepo\qudi-core\src\qudi\core\config\config.py", line 121, in load_config
      config = self._load_config_file(file_path)
    File "C:\Users\t\OneDrive\_Promotion\Software\qudi_newrepo\qudi-core\src\qudi\core\config\file_handler.py", line 42, in _load_config_file
      validate_config(config)
    File "C:\Users\t\OneDrive\_Promotion\Software\qudi_newrepo\qudi-core\src\qudi\core\config\validator.py", line 58, in validate_config
      DefaultInsertionValidator(__cfg_schema).validate(config)
    File "C:\Users\t\.conda\envs\qudicore\lib\site-packages\jsonschema\validators.py", line 353, in validate
      raise error
  jsonschema.exceptions.ValidationError: Additional properties are not allowed ('default_position_unit_prefix', 'image_axes_padding' were unexpected)

  Failed validating 'additionalProperties' in schema['properties']['gui']['additionalProperties']:
      {'additionalProperties': False,
       'properties': {'allow_remote': {'default': False, 'type': 'boolean'},
                      'connect': {'additionalProperties': {'type': 'string'},
                                  'default': {},
                                  'type': 'object'},
                      'module.Class': {'pattern': '^\\w+(\\.\\w+)*$',
                                       'type': 'string'},
                      'options': {'additionalProperties': True,
                                  'default': {},
                                  'type': 'object'}},
       'required': ['module.Class'],
       'type': 'object'}

  On instance['gui']['scanner_gui']:
      {'connect': {'data_logic': 'scanning_data_logic',
                   'optimize_logic': 'optimize_logic',
                   'scanning_logic': 'scanning_probe_logic'},
       'default_position_unit_prefix': None,
       'image_axes_padding': 0.02,
       'module.Class': 'scanning.scannergui.ScannerGui'}

Also, while migrating my old cfg file, I managed to create one that crashed qudi on startup without any fallback. .

Neverhorst commented 2 years ago

No, the qudi-core module does not include any dummy modules that's why the "true default" is always a qudi instance without any modules configured.

Any other config to load is saved (if successful) in the appdata to be used on next startup (if still present, otherwise fallback to empty cfg). It is possible to override the default config by placing a file named default.cfg in the default config directory (<user home>/qudi/config/.

Regarding the exception: Is the traceback you posted coming from the attached cfg file?

Neverhorst commented 2 years ago

@timoML Well, while testing your attached config I saw that there are a lot of systax errors (indentation mainly). Fixed it and now it's working. See here

timoML commented 2 years ago

Some comment without having a strong opinion on it: Is the use of multiple mixins that are employed only once not conflicting with the KISS principle a little...? After all, the functionaliy could live in the Configurationclass for the moment.

timoML commented 2 years ago

@timoML Well, while testing your attached config I saw that there are a lot of systax errors (indentation mainly). Fixed it and now it's working. See here

Yes, my point was rather that also in the case of very bad files (just like with a bit bad ones), the fallback should kick it.

Neverhorst commented 2 years ago

Some comment without having a strong opinion on it: Is the use of multiple mixins that are employed only once not conflicting with the KISS principle a little...? After all, the functionaliy could live in the Configurationclass for the moment.

Well I added them because the Configuration class is self-sufficient. The two mixins just add a more convenient interface to it.

Neverhorst commented 2 years ago

@timoML Well, while testing your attached config I saw that there are a lot of systax errors (indentation mainly). Fixed it and now it's working. See here

Yes, my point was rather that also in the case of very bad files (just like with a bit bad ones), the fallback should kick it.

In case of a config file that fails JSON schema validation, the default kicks in as intended (and also informs you about it in the log and error traceback). The reason why it completely crashed in your case was, that it was not even right YAML syntax, which caused yaml_load to fail and this would cause qudi to crash even in the current main (happens before the new config checking etc.).

But you are right. We can introduce exception handling in this PR as well.

timoML commented 2 years ago

No, the qudi-core module does not include any dummy modules that's why the "true default" is always a qudi instance without any modules configured.

Any other config to load is saved (if successful) in the appdata to be used on next startup (if still present, otherwise fallback to empty cfg). It is possible to override the default config by placing a file named default.cfg in the default config directory (<user home>/qudi/config/.

Regarding the exception: Is the traceback you posted coming from the attached cfg file?

If this is intended behavior, I would find something like Invalid qudi configuration file specified. Falling back to default config. In file <xxx> the follwoing error occured while parsing: easier to grasp. RIght now, it's hard to say whether the traceback is coming from the original or the default config.

Neverhorst commented 2 years ago

No, the qudi-core module does not include any dummy modules that's why the "true default" is always a qudi instance without any modules configured. Any other config to load is saved (if successful) in the appdata to be used on next startup (if still present, otherwise fallback to empty cfg). It is possible to override the default config by placing a file named default.cfg in the default config directory (<user home>/qudi/config/. Regarding the exception: Is the traceback you posted coming from the attached cfg file?

If this is intended behavior, I would find something like Invalid qudi configuration file specified. Falling back to default config. In file <xxx> the follwoing error occured while parsing: easier to grasp. RIght now, it's hard to say whether the traceback is coming from the original or the defualt config.

It does say that in fact:

Invalid qudi configuration file specified. Falling back to default config.
Traceback (most recent call last):
  File "C:\Software\qudi-core\src\qudi\core\application.py", line 118, in __init__
    self.configuration.load_config(config_file, set_default=True)
  File "C:\Software\qudi-core\src\qudi\core\config\config.py", line 127, in load_config
    self.set_config(config)
  File "C:\Software\qudi-core\src\qudi\core\config\config.py", line 61, in set_config
    _validate_config(new_config)
  File "C:\Software\qudi-core\src\qudi\core\config\validator.py", line 60, in validate_config
    DefaultInsertionValidator(config_schema()).validate(config)
  File "C:\Software\qudi-venv\lib\site-packages\jsonschema\validators.py", line 249, in validate
    raise error
jsonschema.exceptions.ValidationError: Additional properties are not allowed ('optimizer_plot_dimensions' was unexpected)

...

And the long traceback from the JSON validator is telling you exactly what's wrong:

...

Failed validating 'additionalProperties' in schema['properties']['gui']['additionalProperties']:

...

On instance['gui']['dummy_confocal']:

...
timoML commented 2 years ago

No, the qudi-core module does not include any dummy modules that's why the "true default" is always a qudi instance without any modules configured.

At least in the current main, the default.cfg comes with the taskrunner gui. Not sure whether this is really needed, though.

Neverhorst commented 2 years ago

Ok, the YAML parser exception is now handled as well.

Neverhorst commented 2 years ago

No, the qudi-core module does not include any dummy modules that's why the "true default" is always a qudi instance without any modules configured.

At least in the current main, the default.cfg comes with the taskrunner gui. Not sure whether this is really needed, though.

While you are of course right, the taskrunner did not have any real tasks to run without installed addons. That's why it was always raising exceptions when trying to run a configured task in pure qudi-core and that's also why I removed it from the default config. As soon as we have a proper install routine in place, I was hoping the installed module addons would place default.cfg files in the userdata dir in order to provide a proper default config depending on what was installed last.

Neverhorst commented 2 years ago

@timoML I took your remark about the Mixins to heart and thought about it. You have a point that the term "Mixin" is misleading in this case. So I refactored the code a bit to be more "base class like" with clear naming of the former Mixin classes. Could you please have a quick look at it again?