Drop Acquisition type restriction from CT filters

Issue #856 resolved
Ed McDonagh created an issue

I’m not sure how well they work, and I think they are not required with the number of events filters.

Proposal: remove them from the interface and code.

Opinions please, particularly from @David Platten as he implemented the acquisition type restriction filters.

Comments (17)

  1. Ed McDonagh reporter

    @David Platten - this is done now apart from adding in new tests and updating the docs.

    Can you tell me if the statement about acquisition level charts (https://docs.openrem.org/en/latest/i_navigate.html) was still true with the new charts, and if I somehow need to re-implement this?

    Once change from the tick box method to the drop down method, is with the tick boxes it was ‘any with axial’ OR ‘any with helical’ (if those two were ticked), but if you select ‘axial: some’ and ‘helical: some’, then only studies with >0 axial series AND >0 helical series would result.

  2. David Platten

    @Ed McDonagh the statement is no longer true. For example, if “Num. axial events” is set to “>0” then the filtered list shows all studies with at least one axial event. Any acquisition-level charts show data for all acquisitions in the filtered studies, whether they are axial or not. I think this behaviour is OK because the charts are simply displaying the data that is in the filtered list.

  3. David Platten

    It worked before using some code in ct_plot_calculations within views.py. The code (version 0.10.0, lines 1198 - 1217) checks to see if ct_acquisition_type is in the form data when an acquisition-level chart is needed. If it is there, then only acquisitions matching the specified type are included in the events from which the chart data is calculated.

  4. David Platten

    This could be implemented for acquisition-level CT charts in the 1.0.0 development branch by adding some logic to the section of views_charts_ct.py which creates the acquisition-level DataFrame. This logic could filter the f.qs before the DataFrame is created.

  5. Ed McDonagh reporter

    If we do implement this for 1.0, we’d want to add the option in the charts section of the right bar, not in the filter bit where I don’t think the behaviour would be intuitive.

    Shall we park acquisition type restricted charts for now, consider it again for 1.1?

  6. Log in to comment