Improvements to `Database`

ebrahimebrahim commented 2 months ago

Eventually the inner workings of Database are likely to be replaced. But for now these improvements help to support the work on https://github.com/OpenwaterHealth/SlicerOpenLIFU/issues/4

Close OpenwaterHealth/OpenLIFU-python#79

Todo

[x] Ensure that dates are being parsed according the format we want. See the options listed in the description of OpenwaterHealth/opw_neuromod_sw#4
[x] Consider adding a unit test that fails before and passes after some of the more important changes in this PR

Notes

empty list of markers

In the example data, I had to change example_session.json in order for Session loading to work here.

In example_session.json we have

  "markers": {
    "id": [],
    "name": [],
    "color": [],
    "radius": [],
    "position": [],
    "dims": [],
    "units": []
  },

which looks like this is trying to be an empty list of markers. But the openlifu Session represents markers as a list of Points, so it should be:

  "markers": [],

to represent an empty list of markers.

volume data versus volume ID

For this to work, another change to example_session.json is to replace the key "volume" by "volume_id".

This is because I have adjusted the Session API here to allow a Session to track a volume ID rather than insisting on loading the actual volume data. This way, the user of the Session api can decide for themselves how they want to handle volume file I/O.

Besides, in the example data the example_session.json already provides a volume ID rather than the actual volume data.

peterhollender commented 2 months ago

One of the reasons for attaching the Volume was that I really want the database i/o to live at the periphery of the code. The Database object hasn't been typically passed around to other methods, as a Session object was sort of "fully-hydrated". However, I think it could work in this instance, because I did later introduce the concept of a Scene, which is a spatially-oriented view of the data from a Session. It could be in the conversion of Session to Scene that the volume is loaded. In the Python code, Session is defined in the db module already, so it makes me a little more comfortable having that operation involve the Database object

ebrahimebrahim commented 2 months ago

One of the reasons for attaching the Volume was that I really want the database i/o to live at the periphery of the code. The Database object hasn't been typically passed around to other methods, as a Session object was sort of "fully-hydrated". However, I think it could work in this instance, because I did later introduce the concept of a Scene, which is a spatially-oriented view of the data from a Session. It could be in the conversion of Session to Scene that the volume is loaded. In the Python code, Session is defined in the db module already, so it makes me a little more comfortable having that operation involve the Database object

That makes sense. In a way, the internals of Session and Subject are part of that database i/o periphery.

Besides volumes, another reason for passing the Database along when constructing a Session is to allow for it to look up the Transducer. The json file defining a Session only has a reference to a transducer, and without the Database object the Session.from_file method wouldn't know how to find the actual transducer definition file.

peterhollender commented 2 months ago

One of the reasons for attaching the Volume was that I really want the database i/o to live at the periphery of the code. The Database object hasn't been typically passed around to other methods, as a Session object was sort of "fully-hydrated". However, I think it could work in this instance, because I did later introduce the concept of a Scene, which is a spatially-oriented view of the data from a Session. It could be in the conversion of Session to Scene that the volume is loaded. In the Python code, Session is defined in the db module already, so it makes me a little more comfortable having that operation involve the Database object

That makes sense. In a way, the internals of Session and Subject are part of that database i/o periphery.

Besides volumes, another reason for passing the Database along when constructing a Session is to allow for it to look up the Transducer. The json file defining a Session only has a reference to a transducer, and without the Database object the Session.from_file method wouldn't know how to find the actual transducer definition file.

Yeah, that tracks. In the MATLAB version, there is no Session.from_file, as it relies on the Database.load_session to load the session data from disk and hydrate it with the transducer and volume.

ebrahimebrahim commented 1 month ago

I added the example database folder as a resource to be used for unit testing. I removed any large files from there and shrank the example volume so this should not bloat the repo

OpenwaterHealth / OpenLIFU-python