Issue445MakeModalityFlexible

#84 Merged at 528a93d
Repository
Branch
issue445MakeModalityFlexible
Repository
Branch
develop
Author
  1. David Platten
Reviewers
Description
  • Refs #445 Make modality more flexible

    So far only implements changes needed in the extractor. Additional changes should be made to 1. make it possible for the user to set "user_defined_modality" 2. update already imported records 3. update "display name page" to get system in correct modality category

    Would also be a workaround for the "unsolvable" issue #444

  • Refs #445 Make modality more flexible

    Implements basic possibility to set a user defined modality type. Only changes from DX to RF and vice versa are allowed. Menu item caption might need to be updated.

    Would also be a workaround for the "unsolvable" issue #444

  • Closed off table tag and slightly altered the wording. References issue #445

Comments (25)

  1. David Platten author

    @edmcdonagh, there was some discussion around whether to use "blank=True" or "null=True" for the new field. It seems there isn't a definitive answer. The present code works fine on my PostgreSQL-based system.

  2. Ed McDonagh

    So this is set to go @dplatten and @LuukO - who wants to have a go at updating the docs to reflect the new changes?

    If you make any changes to the rst files in this branch, they will be automatically built when you commit and can be seen at http://docs.openrem.org/en/issue445makemodalityflexible/ - or at least they would be if RTD wasn't having a funny five minutes with permissions!

  3. Luuk

    I will give it a try. I wonder if the name of the menu-option should be changed, something like "modality properties". The current name doesn't cover the options and I can imagine there will be more options in future. What do you think @edmcdonagh and @dplatten?

    The screenshots should also be retaken. Is a database with the anonimised modalities ("hospital A, scanner 1") available?

  4. David Platten author

    Hi @LuukO. I think that the screenshots are from the developer demo site at http://devdemo.openrem.org/openrem/. The username and password are both "demo". I'll try and update the site to reflect the current develop version at some point in the next few days.

  5. Ed McDonagh

    Thanks @LuukO - and welcome to the world of OpenREM documentation!

    @dplatten, @LuukO do you have an opinion on whether the menu option should be renamed?

  6. Luuk

    I think it should as the name doesn't cover the options anymore (but I gave my opinion already a few comments ago).

  7. David Platten author

    I'm happy for the menu item to be changed. As Luuk says the current name doesn't really reflect what it can do now.

  8. David Platten author

    I like your suggestion, @edmcdonagh. The alternative would be to add another menu item that links to the same page as the display name option, perhaps called Modality config

    On a similar note, I think we should try and standardise on our wording. In the Config menu we currently use View and edit..., ... settings and ... configuration. Perhaps just use ... settings?

  9. Ed McDonagh

    Sounds good to me. Will one of you make that change in this branch and we can have a look?

  10. Ed McDonagh

    System level config menu would be:

    • Manage users
    • Display names & modality settings
    • DICOM object delete settings
    • DICOM networking settings
    • Patient ID settings
    • Skin dose map settings

    I wonder if the word settings becomes unnecessary...

    1. Luuk

      Could also change the heading from 'system level config' to 'system level settings' and have the following items:

      • Users
      • Display names & modality
      • DICOM object deletion
      • DICOM networking
      • Patient ID
      • Skin dose map
  11. Ed McDonagh

    Fast work!

    At some point (before release) we'll need to go through the rest of the docs and replace the images for the other items on that menu.

    One final thing before we merge - it is currently possible to try and change the modality type of CT or Mammo. It fails with a nice message, but is that desirable? Or is it just pragmatic to leave that as it currently is and fix some other stuff?

  12. David Platten author

    You can select multiple display names across modalities at the moment. You may want to change all the selected entries to the same display name, and also change all of the selected DX to RF at the same time. You can do this at the moment. It's unlikely someone would want do to this, but they could if they wanted.

    If we changed the updating page code to prevent the display of the "change modality" box for some modalities then the above wouldn't be possible.

  13. Ed McDonagh

    Can you merge it in then David when you get a chance? Thanks @dplatten, thanks @LuukO

  14. David Platten author

    Would it also be useful to have the display name pages show the inherent modality that each system has? In my live system I have a whole load of systems listed under "Other", and I'm not sure what modality they are. These include multiple entries for one of our CT scanners...

  15. Ed McDonagh

    Plus changes/CHANGES update please! This one will probably feature in the release notes too, but that can wait if you don't want to do it now.

  16. Ed McDonagh

    That 'other' section is something I have been meaning to look at forever. It generally indicates failed imports of some sort or another. It would be good to be able to use the interface to work out what they are, and at the very least delete them! That's probably another issue. I'd also like to be able to delete entire 'modalities' on the basis of the display names grouping...