pace-neutrons / Horace

Horace is a suite of programs for the visualization and analysis of large datasets from time-of-flight neutron inelastic scattering spectrometers.
https://pace-neutrons.github.io/Horace/stable/
GNU General Public License v3.0
7 stars 4 forks source link

Property mandatory_field_set_ in detector classes leads to problems #1071

Open tgperring opened 1 year ago

tgperring commented 1 year ago

To ensure that a class constructor with non-zero number of arguments is called with values for particular properties, a private property has been introduced into most (if not all) instrument classes. This can cause problems because the default object is created with the logical value false for those properties; if a property value is set internally via the private (i.e. underscore) mirror property setter then the corresponding element in mandatory_fieldset is not updated. This has been found to cause unexpected errors on at least one instance in practice. To be certain of avoiding problems requires that properties internally are always set via the dependent property, which in general can be inefficient as this will call check_combo_arg as well as intrinsic Matlab overheads. It is also error prone when updating of classes with additional fields as the indexing elements of mandatory_fieldset to properties is hardwired in all the individual setter methods.

As the reason for introducing mandatory_fieldset is only ensure the constructor is used correctly, it is proposed to extend the functionality of set_positional_and_key_val_arguments (a utility only used in class constructors) to accept a logical array of which saveable fields are mandatory in the constructor argument list. This can be done by allowing the input argument support_dashed_keys to set_positional_and_key_val_arguments to be turned into a structure so that it becomes an options structure. Backwards compatibility is maintained within set_positional_and_key_val_arguments by first checking if it is not a structure, in which case the current interpretation will be maintained.

tgperring commented 1 year ago

The changes have been made to serializable/set_positional_and_key_val_arguments and the proposed changes made for all the detector classes, as part of PR #1143. The use of mandatory fields still needs to be changed for instrument component classes.

cmarooney-stfc commented 11 months ago

@cmarooney-stfc to close when PR is available