Query Retrieve RDSRs with -sr option moves all series of a study

Issue #607 new
Luuk created an issue

When performing a query retrieve with the (advanced) option of only 'SR' OpenREM actually moves all series of a study.

This is caused by a bug in _check_sr_type_in_study. The variable series_sr is a filtered list, which doesn't mean that all other series are removed (they are just filtered out). So walking through this filtered list, doesn't remove the unwanted series.

The code should be (at least this is an option, there might be a better way that removes all filtered out series):

def _check_sr_type_in_study(assoc, study, query_id):
    Checks at an image level whether SR in study is RDSR, ESR, or something else (Radiologist's report for example)
    :param study: Study to check SR type of
    :return: String of 'RDSR', 'ESR', or 'no_dose_report'
    # select series with modality SR
    series_sr = study.dicomqrrspseries_set.filter(modality__exact='SR')
    logger.info(u"Number of series with SR {0}".format(series_sr.count()))
    sopclasses = set()
    for sr in series_sr:
        _query_images(assoc, sr, query_id)
        images = sr.dicomqrrspimage_set.all()
        if images.count() == 0:
            logger.debug(u"Oops, series {0} of study instance UID {1} doesn't have any images in!".format(
                sr.series_number, study.study_instance_uid))
        sr.sop_class_in_series = images[0].sop_class_uid
        logger.info(u"studyuid: {0}   seriesuid: {1}   nrimages: {2}   sopclasses: {3}".format(
            study.study_instance_uid, sr.series_instance_uid, images.count(), sopclasses))
    logger.info(u"sopclasses: {0}".format(sopclasses))
    series_all = study.dicomqrrspseries_set.all() # <-- added
    if '1.2.840.10008.' in sopclasses:
        for sr in series_all: # <-- changed
            if sr.sop_class_in_series != '1.2.840.10008.':
                logger.debug(u"Have RDSR, deleting non-RDSR SR")
        return 'RDSR'
    elif '1.2.840.10008.' in sopclasses:
        for sr in series_all: # <-- changed
            if sr.sop_class_in_series != '1.2.840.10008.':
                logger.debug(u"Have ESR, deleting non-RDSR, non-ESR SR")
        return 'ESR'
        for sr in series_all:
            logger.debug(u"Deleting non-RDSR, non-ESR SR")
        return 'no_dose_report'

Comments (16)

  1. Ed McDonagh

    Thanks for putting this in @LuukO. Can you put the code in a branch so I can do a comparison please?

  2. Ed McDonagh

    Thanks for doing the branch @LuukO.

    The test are failing (hoorah for automated testing!) because this function is used to work out if there are SR series in a study that are preferable to getting the DX or CR images, or SR series that might be better than a Philips 'Dose Info' series etc. In these circumstances, if the it is just he SR series that should be processed, not all the series.

    So we need a different solution. I'll take a look.

  3. Ed McDonagh

    Ok, so I have looked through what is happening.

    First thing is to check that this should be doing what you think it is supposed to do!

    The idea of this flag is for the unusual situation where you have a collection of SRs without the corresponding images. So when you ask for CT, you get nothing, even though your 'PACS' has a load of CT study RDSRs.

    What should happen (but doesn't) is that if you use this flag against a normal set of studies, it should return nothing. In _query_for_each_modality (probably), it should do a query for ModalitiesInStudy = SR, then reject any that also have anything else., which would normally be all the studies.

    Once you have a list of studies that contain only SR, we should then go into _check_sr_type_in_study to find if they are useful SR or not.

    Does that make any sense? (Very tired, sleep deprived due to cars being smashed up with baseball bats and attempts to kill with a vehicle outside my house in the early hours... going to bed now!)

  4. Luuk reporter

    Ok, so the option is not what I thought.

    I actually only wanted to receive rdsrs, but the PACS has also the image data of course and I didn't want to retrieve those. So there is no option to retrieve only rdsrs from a PACS also containing image data? If a study doesn't contain a rdsr, you will always get the image data, correct? Till now I used my own batch script for receiving rdsrs only, but since 0.8 includes a more sophisticated retrieve option.....

  5. Ed McDonagh

    Ha! I need to write the documentation so you don't have to guess! That'll be in issue ref #506.

    The idea is that it will get the best data it can.

    • If for a particular study there is an RDSR, that is the only thing it will get.
    • If it is CT with no RDSR but with series descriptions, it will get just series called 'Dose Info' (Philips) - unless you use the Toshiba flag
    • If it is CT with no RDSR and doesn't return series descriptions, it will get any series with less than 5 images in
    • If it is RF or XA, with no RDSR it won't retrieve anything
    • If it is MG with no RDSR it will retrieve the images
    • If it is CR or DX with not RDSR it will retrieve images

    If that is still getting images you don't want to consider from a modality with no RDSRs, then you could use the station name to exclude that system, as long as the PACS returns station names.

    Does that look like it should work for your situation? What is is you are trying to do?

  6. Ed McDonagh

    This will all need to be revamped again when we upgrade to pydicom 1.0 and pynetdicom3 (whatever the release version ends up being), which will bring in much better progress information during the C-FIND and C-MOVE.

    So if there is another mode you'd like the Q-R to work in, maybe 'RDSR or nothing', then do say and we can maybe incorporate this into the next version (not 0.8).

    The -sr option was really because I had lots of RDSRs on my laptop without the corresponding studies, and I needed to be able to test against something!

  7. Luuk reporter

    I actually think the logic you describe is fine. I tried to get only rdsr, but that's actually covered in a better way (no big CT or fluoroscopy series, but still get dose information in de "images"). Although I think you can get rather big mammography tomosynthesis datasets.

  8. Ed McDonagh

    No, don't close the issue as the code doesn't actually work as intended, even if that is my intention rather than yours!

  9. Ed McDonagh

    You are right about tomosynthesis - are you suffering from the vendor pretending that RDSR should be an optional (costed) extra? Or do you have a system that doesn't do RDSR?

    We made sure when we got our newer units that the older one had the licence added at the same time so all of them use RDSR now.

    I guess in theory people might have chest tomosynthesis units that I assume would suffer in the same way? I don't know what sort of objects they use?

  10. Ed McDonagh

    I think I will push this issue to future for now - it probably isn't worth trying to set up the testing I'd need to confirm my changes at this stage.

  11. Ed McDonagh

    When used as intended, doesn't ignore studies that have other objects other than SR when strictly it should.

    Documentation should clear up the intended usage, maybe a modification to the text in the web interface.

  12. Ed McDonagh

    This should work if ModalitiesInStudy is returned...

        if modalities_returned and inc_sr:
            logger.info(u"Modalities_returned is true and we only want studies with only SR in; removing everything else.")
            for study in study_rsp:
                mods = study.get_modalities_in_study()
                if mods != ['SR']:

    However, it happens quite early on, so relies on the remote node returning the ModalitiesInStudy optional tag. If your PACS doesn't do this, the snippet above won't be run. It would be better to let the other filtering happen then run this code when we already know what the modalities in the study are.

  13. Log in to comment