Output has status: fail despite suppression

[JostMigenda]

I’ve been trying out the demo notebook acro_demo_v2.ipynb with ACRO v0.4.8. In the final cell, acro.finalise(…) generates the following output:

INFO:acro:records:
uid: cross_tabulation
status: fail
type: table
properties: {'method': 'crosstab'}
sdc: {'summary': {'suppressed': True, 'negative': 0, 'missing': 0, 'threshold': 4, 'p-ratio': 0, 'nk-rule': 0, 'all-values-are-same': 0}, 'cells': {'negative': [], 'missing': [], 'threshold': [[2, 0], [2, 1], [2, 2], [4, 0]], 'p-ratio': [], 'nk-rule': [], 'all-values-are-same': []}}
command: safe_table = acro.crosstab(df.recommend, df.parents)
summary: fail; threshold: 4 cells suppressed; 
outcome: parents      great_pret  pretentious        usual
recommend                                        
not_recom            ok           ok           ok
priority             ok           ok           ok
recommend   threshold;   threshold;   threshold; 
spec_prior           ok           ok           ok
very_recom  threshold;            ok           ok
output: [parents     great_pret  pretentious   usual
recommend                                  
not_recom       1440.0       1440.0  1440.0
priority         858.0       1484.0  1924.0
recommend          NaN          NaN     NaN
spec_prior      2022.0       1264.0   758.0
very_recom         NaN        132.0   196.0]
timestamp: 2025-02-24T18:10:54.569102
comments: ['Suppression has been applied. Please let me have this data.']
exception: 

The status of the record above is: fail.
Please explain why an exception should be granted.

Suppression was applied successfully and I have confirmed that the suppressed cells are not included in the output files. However, the status is still set to fail and acro.finalise(…) requires me to explain why an exception should be granted.

Shouldn’t correctly suppressed data have status pass and require no exception? Or am I misunderstanding the status field? (I couldn’t find much documentation, beyond the code and demo notebooks.)

[jim-smith]

Hi @JostMigenda agree that the documentation needs improving - somehow recently the auto-generation of docs failed and needs fixing. (@rpreen ?)

The issue of how we should flag records that have failed but then had suppression applied was one we were unsure about.
TRE co-designers in the SACRO project suggested the current setting, but I agree that users effectively have to add something twice.

We aim to have a regular community event to prioritise issues, so if you wanted to edit this issue to propose a different behaviour we would welcome that.

It might certainly make sense to pre-populate the 'exception request' field if suppression has been applied, so the user wouldn't;t get prompted again when they call finalise() .

The pass/fail stats is used to colour the background in the output checkers' viewer, and TRE staff wanted that type of output clearly flagged for quick manual review. Maybe we need a new 'amber' status to flag it for manual review?

[JostMigenda]

My thinking goes as follows:
If I manually remove the problematic rows/columns from a data frame before adding it as an output to acro, the status of that output would be pass. Thus, if I enable acro suppression to get the same output, I’d expect the status to also be pass—the status should only depend on the output data itself, not on whether I do suppression manually or automatically.

Though of course, I was not involved in the initial discussions so I might be missing some reasons in favour of making a distinction. If there is such a reason, I agree with you that a new “amber”/“suppressed” status (or at least pre-populating the exception request) would be an important usability improvement.

(It could even reduce human error: If I have a large notebook with a dozen outputs I want to finalize, and have to copy&paste “Data was suppressed by acro” as the exception reason for almost every one of them, it’s so easy to miss the one output where I didn’t suppress—whether by accident or because I meant to make an argument for a genuine exception. And similarly, if I’m the output checker and have to look over a large number of outputs, it’s helpful to have more complex exception requests clearly distinguished from routine auto-suppressed ones.)

[rpreen]

Currently the pass or fail is simply intended to report the results of the tests. There is a third review status used for unsupported outputs such as when negatives are involved. Whether/what suppression has been applied is added to the metadata so all of the information is already there for the viewer.

I can see how it would be annoying having to manually declare what mitigation was used when that information is already recorded. If we want to avoid prompting the researcher for those situations then we could populate the exception request field or just skip over prompting for failed outputs that have had a mitigation applied since those details are still included in the metadata for the viewer. The viewer could then treat failed outputs with mitigation applied in the same way as a failure with an exception request.

[rpreen]

I think the original intention was that the researcher should add additional information via the exception request about why the suppression/mitigation is appropriate since the tool can't automatically guarantee it is safe.

[rpreen]

@jim-smith what's wrong with the sphinx docs? It looks like it's working.

I agree that it needs improvement though, but then that's what the paper is for - maybe that needs to be extracted into the docs?

[JostMigenda]

Thanks for the detailed responses, I really appreciate it!

For now, just one comment:

I agree that it needs improvement though, but then that's what the paper is for - maybe that needs to be extracted into the docs?

As long as the paper is up to date with the code, I think it’s sufficient to point people at the paper. However, it would be good to highlight the paper in the README and docs—your comment was the first mention of the paper that I’ve seen. I’ll make a PR to fix that and will then read the full paper before responding to other points.