use_modality_tag not respected in _query_study

Issue #709 resolved
Luuk created an issue

Due to the rewrite of the lines implementing the use_modality_tag are removed. On line 586 in should be added:

    if query.qr_scp_fk.use_modality_tag:
        logger.debug(u'Using modality tag in study query.')
        d.Modality = ''

Also the logic for determining the modality_matching (line 704 and further) has been changed. We might consider changing line 707 to:

                            modality_matching = False or query.qr_scp_fk.use_modality_tag

Btw. I don't know if we had an update or configuration change, but Impax now also responds correctly without "modality at study level". Using above settings both options work for me now (with or without "modality at study level"). We could also remove the option completely, maybe I was the only one suffering from this issue.

Comments (13)

  1. Luuk reporter

    Ed, You are right, I was looking where it might go wrong and thought incorrectly that the use_modality_tag option was removed.

    About the second part: Impax does filter on ModalitiesInStudy, but doesn't return the values (if using the modality tag). So the assumption (modality_matching is false) is incorrect .

    Using the modality_tag we might be better of with some additional queries (the number of modalities minus one) then assuming that modality_matching is False.

  2. Luuk reporter

    Refs Issue #709

    use_modality_tag was respected, but the assumption that modality_matching is false, might be wrong. So if use_modality_tag is true, modality_matching will be true as well. This takes care we always will query for all studies (in this non-conformant situation).

    → <<cset 34ff0f29466a>>

  3. Luuk reporter

    Just tested better. Impax still doesn't return all results if modality tag is not used in a study-level query. With the small change above the query is correctly performed if "use_modality_tag" option is used.

  4. Ed McDonagh

    Attempted to make use_modality_tag exception in modalities_matching clearer. Added suggested option for additional check of having only the right studies as suggested by @LuukO. Refs #709

    → <<cset 39ec6a3be398>>

  5. Log in to comment