Too few station names on OpenREM front page

Issue #239 closed
David Platten created an issue

Constructed from an e-mail conversation between David Platten and Ed McDonagh:

If two rooms have the same station_name then only one entry appears on the OpenREM summary front page for the two rooms. The number of studies shown reflects the total of the two rooms together. When you go to the corresponding filtered page all is well.

A suggested solution is to add a new table (unique_bits_of_kit…) that contains one row per piece of kit that has talked to OpenREM, including a Display name field which is populated with a sensible default (combination of institution_name and station_name, perhaps). Each time a study is added the system can check if it’s a new piece of kit. If it is, a new row is added to unique_bits_of_kit with the appropriate information. The generalequipmentmoduleattr table would need a new field added to it to contain the row of unique_bits_of_kit to use for that study’s Display name. An administrator could be allowed to change the value stored in the Display name field of unique_bits_of_kit.

The unique_bits_of_kit table would need to keep all of the following fields to determine if the kit is unique:

  • Manufacturer
  • Institution Name
  • Institution Address
  • Station Name
  • Institutional Department Name
  • Manufacturer’ Model Name
  • Device Serial Number
  • Software Versions – maybe not?
  • Gantry ID

And then the final field of Display Name, which could be the same for several rows that actually refer to the same bit of kit.

Display Name would also need to be added to each of the filtering searches.

Comments (54)

  1. David Platten reporter

    Added new table to hold unique combinations of kit and associated display name. Added foreign key reference to the general equipment module attr table to link it to the new table. References issue #239.

    → <<cset 04b0dfbbf349>>

  2. David Platten reporter

    @edmcdonagh, does the new table and the new field in the exiting table look OK? Not really sure that I know what I'm doing here...

  3. Ed McDonagh

    Not quite. The class GeneralEquipmentModuleAttr shouldn't need to be edited.

    Instead, your new table needs something like

    general_equipment_module_attributes = models.ForeignKey(GeneralEquipmentModuleAttr)
    

    to make the connection.

    The other thing is that if we are using RDSR, then the ObserverContext table will be populated too. This will have the same information generally as the equipment table, but with the useful addition of the device_observer_uid.

  4. Ed McDonagh

    As much for myself than actually making a comment here:

    On import, we currently store the various identifiers in the GeneralEquipmentModuleAttr table, and there is one row in that table for every single study.

    This new table will only have one row per unique bit of kit.

    On import therefore, we need to compare 9 fields in the new study with each row in the UniqueEquipmentName table until we find a match. And if we don't, we create a new one.

    So it is quite like the remapp.tools.get_values.get_or_create_cid() function, except that was only looking at one field.

    So, given that we have 9 fields, do we

    1. Have a 9 level nested loop of matching (eugh)
    2. Some clever way of matching several fields simultaneously that I don't know of (and haven't looked), or
    3. Create a hash of the combination of fields (not including the display name), store that in a new field, then make the comparison with that instead?
  5. David Platten reporter

    Hi @edmcdonagh. I think that GeneralEquipmentModuleAttr does need to be edited, because each row in that table needs to point to a row in the new table. It can't work the other way around as far as I can see.

  6. David Platten reporter

    Updated definition of new table. Added foreign key field to GeneralEquipmentModuleAttr table. Removed device_observer_uid field from the new table for simplicity at the moment. References issue #239.

    → <<cset 083fe334e784>>

  7. David Platten reporter

    Added new data migration file. This populates the new UniqueEquipmentNames table with every unique set of data currently in the database. Tested and works. Need to add a second stage to this that populates the new unique_equipment_name field in the GeneralEquipmentModuleAttr table with the appropriate row of 'UniqueEquipmentNames'. References issue #239.

    → <<cset e62ff4d4f510>>

  8. Ed McDonagh

    You are right that the GeneralEquipmentModuleAttr needs to be edited to refer to the UniqueEquipmentNames table, not the other way around.

    However, I was envisaging that the GeneralEquipmentModuleAttr table would remain as it is, with a fresh set of data being stored each time, and a new field referring to the appropriate row in the UniqueEquipmentNames table.

    When extracting from an object:

    # ...
        equip.date_of_last_calibration = get_date("DateOfLastCalibration",dataset)
        equip.time_of_last_calibration = get_time("TimeOfLastCalibration",dataset)
        equip.uniquename = get_or_create(manufacturer=equip.manufacturer, institution_name=equip.institution_name, ...)
    

    and I think get_or_create sorts my quandary about matching for us.

  9. David Platten reporter

    I'm not planning on changing GeneralEquipmentModuleAttr, other than adding the reference to the UniqueEquipmentNames table. Thanks for the heads-up about the get_or_create - that can certainly be used by the extractors when dealing with new data.

    I think I do need to add to the data migration that I have put together in order to populate the field in the GeneralEquipmentModuleAttr table with an appropriate reference to UniqueEquipmentNames (just for users that are upgrading).

  10. David Platten reporter

    Added to data migration file. Now adds the appropriate UniqueEquipmentNames foreign key value into every existing row of the GeneralEquipmentModuleAttr table. References issue #239.

    → <<cset b169498b22b2>>

  11. David Platten reporter

    To do list:

    • Add methods in each of the extractor routines to populate the unique_equipment_name field in GeneralEquipmentModuleAttr when new data comes into the system. These methods must check whether the UniqueEquipmentNames table already contains the appropriate unique combination of information. If it does, use the appropriate foreign key value in the unique_equipment_name field of GeneralEquipmentModuleAttr. If it doesn't, create a new row in UniqueEquipmentNames and use the new row's foreign key value. I think this is where @edmcdonagh is suggesting the use of get_or_create.

    • Use the new unique_equipment_name field on the front page of OpenREM so that x-ray systems that have the same station_name appear as separate entries.

    • Use the new unique_equipment_name on the filtered html pages?
    • Does it need to be used anywhere else?
  12. Ed McDonagh

    It definitely needs to be used on the filtered pages (including as a search), as that is where I currently have a problem with modalities that don't have a station name, and the current link from the front page tries to search for station names of "None".

    It might be wise to add it to exports too, but this would be a lower priority.

    As I understand it, the first bullet point is completely handled by simply using get_or_create and passing the fields.

    Presumably though we will be working with unique_equipment_name.display_name or what ever the link is in that context?

  13. David Platten reporter

    If some of your entries are missing a station name then I need to adjust the migration code: the combining of institution and station name fails if one of them is none. I've included a check for a none in institution name, but not for a none in station name. I was assuming station name would always be populated. Will sort next week.

    Also need to add a form that an admin can get to so that the default display names can be edited.

  14. Ed McDonagh

    I had made the same assumption... but I've had one lot of data where the anonymisation was a little vicious, and another where it just isn't there :(

  15. David Platten reporter

    Updated migration file to cope with None in both station_name and institution_name. If they're both blank then the text Blank is used for the dispaly_name. References issue #239.

    → <<cset fb84df00259e>>

  16. David Platten reporter

    Added code to dx.py to get_or_create corresponding UniqueEquipmentNames information. If a new row has to be created in UniqueEquipmentNames then the display_name field is populated with a sensible default, depending on what information is available. Tested by adding some fake DICOM data to a backup of my live system: works as expected. References issue #239.

    → <<cset b419295714d3>>

  17. David Platten reporter

    Added code to mam.py to get_or_create corresponding UniqueEquipmentNames information. If a new row has to be created in UniqueEquipmentNames then the display_name field is populated with a sensible default, depending on what information is available. Untested, but identical to working code in dx.py. References issue #239.

    → <<cset 31d390f35ba0>>

  18. David Platten reporter

    Added code to ct_philips.py and rdsr.py to get_or_create corresponding UniqueEquipmentNames information. If a new row has to be created in UniqueEquipmentNames then the display_name field is populated with a sensible default, depending on what information is available. Untested, but identical to working code in dx.py. References issue #239.

    → <<cset 3034752bf95f>>

  19. David Platten reporter

    Updated openrem_home to use display_name rather than station_name. Works: I now have separate entries for systems that would have previously been combined together as they have the same station_name. References issue #239.

    → <<cset 26f2e21b157d>>

  20. David Platten reporter

    Changes to make dxfiltered.html display correctly using display_name rather than station_name. Further checking required. References issue #239.

    → <<cset 8330b76c53a7>>

  21. David Platten reporter

    @edmcdonagh, at the moment I can't work out how to show display_name values on dxfiltered.html. Other database fields are shown using this type of syntax:

    {{ exam.projectionxrayradiationdose_set.get.accumxraydose_set.get.accumintegratedprojradiogdose_set.get.total_number_of_radiographic_frames }}
    

    I think I need something like:

    {{ exam.generalequipmentmoduleattr_set.get.uniqueequipmentnames_set.get.display_name }}
    

    but that doesn't work. Any suggestions? I don't seem to be able to debug properly in the templates, so I'm struggling to work out how to do this.

  22. Ed McDonagh

    Difficult to answer without having a populated database to walk through in the manage.py shell, but I would think you need something like:

    {{ exam.generalequipmentmoduleattr_set.get.unique_equipment_name.display_name }}
    
  23. David Platten reporter

    Successfully added display_name to the dxfiltered.html page (thanks for the help with the syntax, @edmcdonagh). I'm very happy to alter what is displayed where: this is just a proof-of-concept. References issue #239.

    → <<cset 59c973077992>>

  24. David Platten reporter

    I think that I'm finished with this now. @edmcdonagh, the data migration file to address this issue can be combined with the one that addresses the median issue - what do you think?

  25. David Platten reporter

    Added new view, display_names_view, to display the contents of the UniqueEquipmentNames database table. It's not linked from anywhere yet, but could be added to the admin menu. The admin menu needs to be added to the page, and something needs to be done to the rows to allow the user to edit the display_name of the row that they click on. You get to the page by going to /viewdisplaynames/. References issue #239.

    → <<cset cf13d973f51b>>

  26. David Platten reporter

    Added column to the displayed data. Clicking on this takes the user to a new view that will enable them to edit the of the row that they clicked on. Clicking on this link runs the new method in . At the moment this is simply a duplicate of , with the addition of being able to cope with being sent the primary key of the row that the user wants to update. It simply returns the user to the display of all the data at the moment. References issue #239.

    → <<cset fe42e2a1f330>>

  27. David Platten reporter

    Added new html template to be used to edit the user-selected . When a user clicks to update a they are taken to this new page. At the moment it just displays the row to be updated - it doesn't allow any editing at the moment. The needs to be a form input field so that the user can edit the value and then submit the changes. References issue #239.

    → <<cset 819dc501bd20>>

  28. David Platten reporter

    Nearly there. The user can now click to update a row in the UniqueEquipmentNames table. This takes them to a view of just that row, where the display_name can be edited. Clicking on the Submit button sends the updated data back to display_name_update in views.py. This just needs some code adding for when the request method is POST to process the form data that has been sent, and update the appropriate record in UniqueEquipmentNames. References issue #239.

    → <<cset 301bd878accd>>

  29. David Platten reporter

    Tweaked a few things: added option under User options to view display names; table showing display names is now sorted alphabetically by display_name; made entire table rows clickable on the /viewdisplaynames/ page (they're only clickable if the user has admin rights); the HTML FORM components on the /updatedisplayname/## page are only put in if the user has admin rights; altered where /updatedisplayname/ points to in urls.py to prevent an error if a user manually types in a url without a value at the end. References issue #239.

    → <<cset 5320f97d3154>>

  30. David Platten reporter

    @edmcdonagh, this is working now, but the UniqueEquipmentNames table that I have created does not include the device_observer_uid from the ObserverContext table. Does that matter?

  31. Ed McDonagh

    Hi @dplatten - lets leave it as it is for now, and work out incorporating this if and when what we have isn't enough 😄

  32. Log in to comment