- edited description
Too few station names on OpenREM front page
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)
-
reporter -
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>>
-
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...
-
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.
-
reporter Updated definition of new table in light of @edmcdonagh's comments. References issue
#239.→ <<cset 0e7df11b3876>>
-
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
- Have a 9 level nested loop of matching (eugh)
- Some clever way of matching several fields simultaneously that I don't know of (and haven't looked), or
- 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?
-
reporter Updated definition of new table to include a
unique_together
statement that forces the combination of certain fields to be unique (see https://docs.djangoproject.com/en/1.8/ref/models/options/#unique-together). References issue#239.→ <<cset b80aa5239686>>
-
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.
-
reporter Updated definition of new table. Added foreign key field to
GeneralEquipmentModuleAttr
table. Removeddevice_observer_uid
field from the new table for simplicity at the moment. References issue#239.→ <<cset 083fe334e784>>
-
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 newunique_equipment_name
field in theGeneralEquipmentModuleAttr
table with the appropriate row of 'UniqueEquipmentNames'. References issue#239.→ <<cset e62ff4d4f510>>
-
You are right that the
GeneralEquipmentModuleAttr
needs to be edited to refer to theUniqueEquipmentNames
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 theUniqueEquipmentNames
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.
-
reporter I'm not planning on changing
GeneralEquipmentModuleAttr
, other than adding the reference to theUniqueEquipmentNames
table. Thanks for the heads-up about theget_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 toUniqueEquipmentNames
(just for users that are upgrading). -
reporter Added to data migration file. Now adds the appropriate
UniqueEquipmentNames
foreign key value into every existing row of theGeneralEquipmentModuleAttr
table. References issue#239.→ <<cset b169498b22b2>>
-
reporter Removed an unsed variable and tidied a bit. References issue
#239.→ <<cset 4424495543f4>>
-
reporter To do list:
-
Add methods in each of the extractor routines to populate the
unique_equipment_name
field inGeneralEquipmentModuleAttr
when new data comes into the system. These methods must check whether theUniqueEquipmentNames
table already contains the appropriate unique combination of information. If it does, use the appropriate foreign key value in theunique_equipment_name
field ofGeneralEquipmentModuleAttr
. If it doesn't, create a new row inUniqueEquipmentNames
and use the new row's foreign key value. I think this is where @edmcdonagh is suggesting the use ofget_or_create
. -
Use the new
unique_equipment_name
field on the front page of OpenREM so that x-ray systems that have the samestation_name
appear as separate entries. - Use the new
unique_equipment_name
on the filtered html pages? - Does it need to be used anywhere else?
-
-
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? -
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.
-
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 :(
-
reporter Updated migration file to cope with
None
in bothstation_name
andinstitution_name
. If they're both blank then the textBlank
is used for thedispaly_name
. References issue#239.→ <<cset fb84df00259e>>
-
reporter Added code to dx.py to get_or_create corresponding
UniqueEquipmentNames
information. If a new row has to be created inUniqueEquipmentNames
then thedisplay_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>>
-
reporter Added code to mam.py to get_or_create corresponding
UniqueEquipmentNames
information. If a new row has to be created inUniqueEquipmentNames
then thedisplay_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>>
-
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 inUniqueEquipmentNames
then thedisplay_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>>
-
reporter Updated
openrem_home
to usedisplay_name
rather thanstation_name
. Works: I now have separate entries for systems that would have previously been combined together as they have the samestation_name
. References issue#239.→ <<cset 26f2e21b157d>>
-
reporter Changes to make
dxfiltered.html
display correctly usingdisplay_name
rather thanstation_name
. Further checking required. References issue#239.→ <<cset 8330b76c53a7>>
-
reporter Removed study DAP from DX filters. This references issue
#249. Some small fixes to filtering re study date. References issue#239.→ <<cset 1ad89c0a3a31>>
-
reporter Added filter to all modalities in . References issue
#239.→ <<cset 21f08d02698b>>
-
reporter Modified home page so that equipment is shown by
display_name
rather thanstation_name
. References issue#239.→ <<cset 4ae4b93ac0b2>>
-
reporter Home page now displays the
display_name
in the first column for each modality. References issue#239.→ <<cset cc5df2846b2b>>
-
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.
-
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 }}
-
reporter Thanks @edmcdonagh - that was exactly what I needed. Working now.
-
reporter Successfully added
display_name
to thedxfiltered.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>>
-
reporter Added
display_name
to the other filtered html pages. References issue#239.→ <<cset 9ae6436c914a>>
-
reporter Added
display_name
to the dx exports csv and xlsx files. References issue#239.→ <<cset 1cf211559523>>
-
reporter Added
display_name
to the other export routines, except for the NHSBSP mammo, which I left alone. References issue#239.→ <<cset f0fc753d9ec2>>
-
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?
-
reporter OK - not quite finished. Need to add a view that administrators can use to update the
display_name
values stored in theUniqueEquipmentNames
table. I think I need to use some of this: https://docs.djangoproject.com/en/1.8/ref/class-based-views/generic-editing/ -
That was going to be my reply - but you got there first!
-
reporter Added new view,
display_names_view
, to display the contents of theUniqueEquipmentNames
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 thedisplay_name
of the row that they click on. You get to the page by going to/viewdisplaynames/
. References issue#239.→ <<cset cf13d973f51b>>
-
reporter The admin menu is back: I wasn't passing an admin variable from the view to the template in the return structure. References issue
#239.→ <<cset 92d022ab98cc>>
-
reporter Added display name viewing to the admin menu. References issue
#239.→ <<cset d152be8d5637>>
-
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>>
-
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>>
-
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 thedisplay_name
can be edited. Clicking on theSubmit
button sends the updated data back todisplay_name_update
inviews.py
. This just needs some code adding for when the request method isPOST
to process the form data that has been sent, and update the appropriate record inUniqueEquipmentNames
. References issue#239.→ <<cset 301bd878accd>>
-
reporter - changed status to resolved
Pages for viewing and updating now working. Fixes issue
#239.→ <<cset cf08394a30dd>>
-
reporter - changed milestone to 0.7.0
-
reporter Tweaked a few things: added option under
User options
to view display names; table showing display names is now sorted alphabetically bydisplay_name
; made entire table rows clickable on the/viewdisplaynames/
page (they're only clickable if the user has admin rights); the HTMLFORM
components on the/updatedisplayname/##
page are only put in if the user has admin rights; altered where/updatedisplayname/
points to inurls.py
to prevent an error if a user manually types in a url without a value at the end. References issue#239.→ <<cset 5320f97d3154>>
-
reporter @edmcdonagh, this is working now, but the
UniqueEquipmentNames
table that I have created does not include thedevice_observer_uid
from theObserverContext
table. Does that matter? -
Hi @dplatten - lets leave it as it is for now, and work out incorporating this if and when what we have isn't enough
-
reporter Removed some unused variables. References issue
#239.→ <<cset 92448b4a670a>>
-
reporter I have this working on my live system now and it seems to work very well.
-
Time for a pull request?
-
reporter Yes please.
-
- changed status to closed
Merging @dplatten's pull request #22 with a solution for units that share station names, or don't have one at all. Closes
#239→ <<cset c1fb93d08dd3>>
- Log in to comment