senaite.core
senaite.core copied to clipboard
Support for Multiple component analysis
Description of the issue/feature this PR addresses
This Pull Request adds the Multiple Component analysis. Spectroscopic methods are common analytical methods for which multiple component analysis is used, like are Atomic Absorption Spectroscopy (AAS), Inductively Coupled Plasma Spectroscopy (ICP) or Ultraviolet and Visible (UV). Basically, the concentration of individual components/analytes is detected simultaneously, with a single analyzer.
Creation of an Multicomponent Analysis Service:
Selection of analytes on Sample creation:
Multicomponent analyses in worksheet:
Multicomponent analyses in sample:
Current behavior before PR
SENAITE does not support multi-component analysis. Lab is forced to create an individual Analysis Service for every analyte/component
Desired behavior after PR is merged
SENAITE does support multi-component analysis. Analytes can be defined as components of a single Analysis Service
-- I confirm I have tested this PR thoroughly and coded it according to PEP8 and Plone's Python styleguide standards.
Creating a new Analysis Service with Analytes complains about that the Keyword is already in use, no matter which keyword is chosen:


However, although the error is displayed, the Analysis service is created with that keyword:

And interestingly, it is also updated when the keyword changes (which also causes the error message):

Just a general thought:
You implemented now Analytes as siblings to existing analyses, but I think they should be actually contained inside analyses. I know that we did it the same for sample partitions, but I think that decision should be also rethought...
Creating a new Analysis Service with Analytes complains about that the Keyword is already in use, no matter which keyword is chosen: [...] However, although the error is displayed, the Analysis service is created with that keyword
Addressed with https://github.com/senaite/senaite.core/pull/2120/commits/245354b974a7175b7949ed78f19288bfdddc15db
Just a general thought:
You implemented now Analytes as siblings to existing analyses, but I think they should be actually contained inside analyses. I know that we did it the same for sample partitions, but I think that decision should be also rethought...
Yes, it might make sense. However, making the system work with Analysis objects that are not contained in AnalysisRequest entails a lot of additional changes, not only in ARAnalysisField (plenty of objectValues('Analysis')
in there, but also in logic that is related with Worksheet. The good thing about this approach is that since analytes become regular "Analyses", the existing logic works without any changes other than those related with visualization and guards. I think this discussion will come up again when we move to DX and need to rewrite ARAnalysesField
. Maybe this is something we could reconsider by then.
Some more findings from integration testing:
- Analytes that have the same keyword as existing Analysis services, e.g.
Zn
collide in Worksheets and show only one of both - Transposed worksheets show the Multi Component Analysis in between the Analytes and not on top. Maybe we need to fix the sort key on the fly there?
Assigning a calculation to a multi component disables all result fields of the Analytes.
It is not possible to retract an Analyte result that is in to_be_verified
state
When an Analyte has the same keyword as an Analysis, e.g. Zn
, and the Analysis exists together in the same Sample with the Analyte, it is not possible to remove the Analysis anymore over the "Manage Analyses" view.
All Analytes are added when a sample is copied, although only e.g. 2/5 Analytes were selected in the source samples' multi-component analysis.
Just another thought that came into my mind:
To support all features of Analyses, e.g. LDL/UDL, Uncertainties, Text Result, Option fields etc., wouldn't it make sense to implement analytes as multi-references to existing Analysis Services? Then we would not have the issue with the same keyword and all the other options that can be configured with analysis services.
Just another thought that came into my mind:
To support all features of Analyses, e.g. LDL/UDL, Uncertainties, Text Result, Option fields etc., wouldn't it make sense to implement analytes as multi-references to existing Analysis Services? Then we would not have the issue with the same keyword and all the other options that can be configured with analysis services.
This was my initial thought as well. However, note that there is a single analytical process involved with a multi-component analysis. I mean, the result of such analytical process is the concentrations of analytes/components, precisely. Thus, if they were services you'd need all them to always be configured the same way in most of the configuration settings, like units, interim fields, precision, etc. A multi-component service made of services that are barely similar among them won't make sense at all.
Thus, I thought that instead of putting a lot of efforts to keep different services in-sync because they were assigned to the same multi-component (and also consider the possibility that might be unassigned later) it would be better and simpler if the multi-component service simply acted as the template of the analytes. This is precisely why the analytes are created as Analysis copies of the multi-component analysis they belong to (all analytes inherit same configuration from parent).
Although there are some settings (e.g. results ranges, MDL, etc.) for which we we will need to do some dance (something I wanted to do in another PR, btw), this copy approach I believe makes the things simpler. For instance, my idea re calculations (I saw you noticed this "weak point" already in one of the comments), was that you should be able to assign a calculation to a multi-component service. The calculation would not have any effect to the multi-component analysis though, but to the analytes only. This would be possible just by making the system to not render the interim fields in the analyses listing for multi-component analysis, but for analytes only. Another approach would be to just remove the calculation from the multi-component analysis as soon as the analytes are created. Imagine if you had different services, one per analyte, you should then to assign the same calculation to all them first, etc.. a nightmare imo.
Besides, a single multi-component service can comprehend a long list of analytes. For instance, for the determination of Volatile Organic Components (VOCs) one might define, quite easily, more than 40 or 60 different components/analytes. Also, not rare to have a multi-component analysis with 150 analytes. Imagine if the lab has say 4 different multi-component analyses, with that many components each! Cannot think an easy way to handle and keep all them in-sync.
So, these are the reasons why I decided to create copies of the analytes as Analysis instead of AnalysisService. But yes, I do know there is still work to do, specially regarding ranges and detection limits. I think I can add the calc thing in this PR though.
All Analytes are added when a sample is copied, although only e.g. 2/5 Analytes were selected in the source samples' multi-component analysis.
:sob:
Thanks for the explanation and I agree with the points you mentioned. Maybe we can avoid the keyword clashes when we use the same validator for the analytes and maybe this approach for later:
We allow services to contain Analyte services that are created automatically when you add the rows in the multi component.
Thanks for the explanation and I agree with the points you mentioned. Maybe we can avoid the keyword clashes when we use the same validator for the analytes
Yes, using same validator for analyte keywords as for services makes sense, yes.
... and maybe this approach for later:
We allow services to contain Analyte services that are created automatically when you add the rows in the multi component.
Not sure if I get the point. Even if we "hide" these "Analyte" services, we will always need to keep them up-to-date each time we do configuration changes to the multi-component service they belong to. This would mean to restore that relic of ProxyField or manually update the values of all analytes each time a value from a multi-component service is updated by adding a lot of redundant code in the setters. Don't thing this is more elegant than what we have with this "Analysis"-only approach. Maybe I am missing something though...