Raise a warning (optional error) when bang-strings in cmds are not used?

[hugobuddel]

Some of our notebooks set bang strings in cmds that are not actually used in any effect and thus have no effect.

E.g. the strehl ratio in https://github.com/AstarVienna/irdb/blob/dev_master/MICADO/docs/example_notebooks/2_scopesim_SCAO_1.5mas_astrometry.ipynb , see AstarVienna/irdb#200

Such an unused bang-string in cmds usually is an indication of a bug, either on the user/notebook side, or on the ScopeSim side (or in the IRDB package).

Is it feasible to detect such unused !-strings @teutoburg ?

A proper check will be hard / impossible, but we might be able to do something reasonable. For example, we have a dynamic check on whether the bang-string is ever used in a value elsewhere in the cmds, and have some list of hardcoded bang-strings that ScopeSim accesses directly.

It doesn't have to be fool proof; for now it would be good enough to detect problems in our own notebooks before users discover them. Then maybe later we could also use the check as a fail-safe for users (maybe after we get rid of .meta, if that ever happens).

[teutoburg]

This might not be as easy as it seems at first...

Consider the following case: I want to run the same simulation twice, once with a certain effect included and once without, to compare the results. This effect happens to be the only one that would use a certain cmds keyword. Now when I turn off this effect, the simulation would produce a warning (or even an error) unless I specifically remove that key from the cmds before. I don't think such results should be the goal of this.

Also in terms of implementation, the one thing I could come up with is to have every .__getitem__(key) call on the cmds put the key in a set somewhere as a side-effect and in the end that set is compared to the keys of cmds, which would allow to quickly see which ones are missing. But this would break if e.g. the cmds are printed anywhere (which is done in some of the notebooks and might very well be done by a user) because printing iterates over all keys and thus triggers __getitem__. Unless maybe this whole mechanism becomes part of opt.observe() or opt.readout() and is reset there. But then, in which of the two would we put it? We might have multiple readout calls and maybe not all of them use a particular cmds key? Or maybe something is only used in either observe or readout but the cmds is printed in between? I see many potential practical issues with implementing something like that...

And conceptually, I'm not sure if I agree that it should be "illegal" to have unused keys in the cmds at all. But I don't know, maybe there's a good argument.

[hugobuddel]

Oh, I didn't yet think of that first issue of introducing/removing an effect. I did think of the second one of 'printing' being a problem and then I thought that maybe we only count calls from Effects or so, but that gets way too complicated.

My immediate goal is to prevent us from giving the users broken documentation, and that is feasible I think. The check doesn't have to be perfect, just good enough to ensure we update the notebooks when we update the IRDB package (like switching the default PSF effect). With such a limited scope we can also hardcode some exceptions if necessary.

This problem does seem to provide an interesting frame to think about the UserCommands and .meta and such. E.g. how could we implement the UserCommands and Effect parameters in a way that would make writing such a incorrect-bang-sring check trivial?

Let's just leave this open in Backlog and who first implements something useful wins nothing eternal fame.