Kapton error handling

[Baharis]

Previous implementation of dials/command_line/stills_process.py Processor.integrate and dials/command_line/integrate.py run_integration had an unexpected UnboundLocalError whenever absorption_correction.apply=True, but absorption_correction.algorithm was unspecified (former specifier 2/4 possible states, and later 1/4 possible state). Now, in both places whenever absorption_correction.algorithm is expected but unspecified, a ValueError is raised, unless user specifically requests absorption_correction.algorithm=other.'

This change should not affect any existing processes, but might save a few hours some other poor soul, who will inevitably accidentally declare multiple absorption protocols and unexpectedly ends up with {apply=True, algorithm=None}, like me last week.

[codecov]

Codecov Report

Merging #2317 (8d9dffe) into main (9e9a7f5) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

❗ Current head 8d9dffe differs from pull request most recent head 9b2452e. Consider uploading reports for the commit 9b2452e to get more accurate results

@@            Coverage Diff             @@
##             main    #2317      +/-   ##
==========================================
- Coverage   80.59%   80.54%   -0.05%     
==========================================
  Files         587      587              
  Lines       67135    67144       +9     
  Branches     8950     8954       +4     
==========================================
- Hits        54109    54084      -25     
- Misses      10957    10988      +31     
- Partials     2069     2072       +3     

[ndevenish]

Do other parts of the code use "other" anywhere, or is it just worth removing "other" from the list of possibilities?

I also wonder if apply= could be ignored needing to specify it separately; the presence of algorithm= might be enough to know you want to turn it on?

[Baharis]

I checked and these are the only two places where "other" algorithm is used. It has its use, as it makes it easy to add a custom correction, but anyone capable of writing a custom absorption algorithm will most likely be able to plug it into DIALS themselves, so is it really needed? I am not sure...

In the current state of code apply could be probably ignored and removed altogether, but wouldn't it make it much less obvious how to turn off the correction, if needed?

I briefly spoke with @irisdyoung and another idea to simplify phil here would be to remove .multiple from absorption correction altogether, as there was no instance where it was actually used. Removing .multiple might make the other field useful, should anyone want to easily switch to multiple corrections by minor code modification.

If we are already speaking about kapton phil, then I personally do not understand why the distinction between fuller_kapton and kapton2019 is there, other than for historical reasons. Isn't kapton2019 a generalized version of fuller_kapton? I also have some doubts about the defaults used for numerical parameters, as they were certainly good for one (and exactly one) experiment.

All of these kapton phil modifications could make a good subject of another PR, but here I'd just want to focus on making the Error message clearer.

[ndevenish]

Okay, normally I'd push for the "other" handling to be removed from here (and the PHIL option) since it does nothing (anyone extending it would need to know that it went here anyway), but as a general policy don't touch anything in dials.stills_process - since it's hard to know if anything relies on it being set but doing nothing.