search

commontk / CTK

A set of common support code for medical imaging, surgical navigation, and related purposes.
https://commontk.org
Apache License 2.0
827 stars 480 forks source link

Add Visual DICOM Browser | :warning: Authorship of integration commit changed #1165

Closed jcfr closed 5 months ago

jcfr commented 5 months ago

:warning: Changes originally introduced in commit 8a0717da81ff32cea34a724cfac93a443bbe3caa via this pull request have been removed from the main branch using force push. :warning:

:arrow_right: These changes have been replaced by the commit 88ff72b9.

Rationale:

After performing a Squash & Merge operation in association with this pull request (#1165(, the primary authorship of the commit was inadvertently attributed to @jcfr instead of @Punzo, who contributed significantly to the changes.

To rectify this, a force push was applied to explicitly set @Punzo as the main author of the commit, reflecting their substantial contribution to the work.

This pull request introduces the ctkDICOMVisualBrowserWidget, which improves the threaded execution of various operations:

Key Components:

Related

Testing

Additionally, a CTK application named ctkDICOMVisualBrowser has been included for testing purposes (requires enabling CMake options examples and DICOM).

Testing can also be performed by running the visual DICOM browser from the Slicer Python console:

import os

server = ctk.ctkDICOMServer()
server.connectionName = "ConnectionName"
server.callingAETitle = "callingAETitle"
server.calledAETitle = "calledAETitle"
server.host = "host"
server.port = 11112
server.retrieveProtocol = ctk.ctkDICOMServer.CGET

DICOMDatabase = qt.QSettings().value(slicer.dicomDatabaseDirectorySettingsKey)
if not os.path.isdir(DICOMDatabase):
  os.makedirs(DICOMDatabase)  

browser = ctk.ctkDICOMVisualBrowserWidget()
browser.addServer(server)
browser.setDatabaseDirectory(DICOMDatabase)
browser.filteringPatientID = 1
browser.resize(slicer.util.mainWindow().size)
browser.show()
browser.onQueryPatient()

Similarly, here is how to test the scheduler from the Python console (e.g. query and retrieve):

server = ctk.ctkDICOMServer()
server.connectionName = "ConnectionName"
server.callingAETitle = "callingAETitle"
server.calledAETitle = "calledAETitle"
server.host = "host"
server.port = 11112
server.retrieveProtocol = ctk.ctkDICOMServer.CGET

scheduler = ctk.ctkDICOMScheduler()
scheduler.setDicomDatabase(slicer.dicomDatabase)
scheduler.addServer(server)

nDays = 30
endDate = qt.QDate().currentDate()
startDate = endDate.addDays(-nDays);
parameters = {
  "ID": "CT",
  #"Name": "name",
  #"Study": "description",
  #"Series": "description",
  #"Modalities": ["CT", "MR"],
  #"StartDate": startDate.toString("yyyyMMdd"),
  #"EndDate": endDate.toString("yyyyMMdd")
}

scheduler.setFilters(parameters)
# Use one of the following methods: all these methods run background processes!!!!
#scheduler.queryPatients()
#scheduler.queryStudies('patientID')
#scheduler.querySeries('patientID', 'studyInstanceUID')
#scheduler.queryInstances('patientID', 'studyInstanceUID', 'seriesInstanceUID')
#scheduler.retrieveStudy('patientID', 'studyInstanceUID')
#scheduler.retrieveSeries('patientID','studyInstanceUID', 'seriesInstanceUID')
#scheduler.retrieveSOPInstance('patientID', 'studyInstanceUID', 'seriesInstanceUID', 'SOPInstanceUID')
UML diagram Screenshot
VisualDICOMBrowserUML ScreenshotVisualDICOMBrowser
ScreenshotSettings

Video:

https://github.com/commontk/CTK/assets/7985338/a0e362e7-858a-4e47-9298-8d00ae16a23b

jcfr commented 5 months ago

Proposed commit title & message for Squash & Merge:

ENH: Add Visual DICOM Browser

Introduces the `ctkDICOMVisualBrowserWidget`, which improves the threaded
execution of the following operations:

- Filtering and navigation with thumbnails of local database and servers results
- Import from file system to local database
- Query/Retrieve from servers (DIMSE C-GET/C-MOVE )
- Storage listener
- Send (emits only a signal for the moment, requires external implementation)
- Remove (only from local database, not from server)
- Metadata exploration

In addition, the commit introduces the following classes:

* `Core` classes:
  * `ctkAbstractJob`
  * `ctkAbstractScheduler`
  * `ctkAbstractWorker`
* `DICOM/Core` classes:
  * `ctkDICOMEcho`
  * `ctkDICOMInserter`
  * `ctkDICOMInserterJob`
  * `ctkDICOMInserterWorker`
  * `ctkDICOMJob`
  * `ctkDICOMJobResponseSet`
  * `ctkDICOMQueryJob`
  * `ctkDICOMQueryWorker`
  * `ctkDICOMRetrieveJob`
  * `ctkDICOMRetrieveWorker`
  * `ctkDICOMScheduler`
  * `ctkDICOMServer`
  * `ctkDICOMStorageListener`
  * `ctkDICOMStorageListenerJob`
  * `ctkDICOMStorageListenerWorker`
* `DICOM/Widget`classes:
  * `ctkDICOMPatientItemWidget`
  * `ctkDICOMSeriesItemWidget`
  * `ctkDICOMServerNodeWidget2`
  * `ctkDICOMStudyItemWidget`

Co-authored-by: Andras Lasso <lasso@queensu.ca>
Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

cc: @lassoan @Punzo

jcfr commented 5 months ago
Warnings to fix: ``` /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMPatientItemWidget.cpp: In member function ‘void ctkDICOMPatientItemWidget::onSeriesItemClicked()’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMPatientItemWidget.cpp:791:35: warning: enum constant in boolean context [-Wint-in-bool-context] 791 | (Qt::ControlModifier || Qt::ShiftModifier)) | ^~~~~~~~~~~~~ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMPatientItemWidget.cpp:791:35: warning: enum constant in boolean context [-Wint-in-bool-context] [ 89%] Building CXX object Libs/DICOM/Widgets/CMakeFiles/CTKDICOMWidgets.dir/ctkDICOMThumbnailListWidget.cpp.o /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp: In member function ‘virtual bool QCenteredStyledItemDelegate::editorEvent(QEvent*, QAbstractItemModel*, const QStyleOptionViewItem&, const QModelIndex&)’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp:171:59: warning: passing ‘Qt::CheckState’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 171 | return model->setData(index, state, Qt::CheckStateRole); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp:171:59: warning: in call to ‘QVariant::QVariant(int)’ [-Wsign-promo] [ 89%] Building CXX object Libs/DICOM/Widgets/CMakeFiles/CTKDICOMWidgets.dir/ctkDICOMVisualBrowserWidget.cpp.o /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp: In member function ‘void ctkDICOMServerNodeWidget2::saveSettings()’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp:1001:91: warning: passing ‘bool’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 1001 | settings.setValue("DICOM/StorageEnabled", QString::number(this->storageListenerEnabled())); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMServerNodeWidget2.cpp:1001:91: warning: in call to ‘static QString QString::number(int, int)’ [-Wsign-promo] /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp: In member function ‘void ctkDICOMVisualBrowserWidgetPrivate::init()’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:373:136: warning: passing ‘ctkDICOMVisualBrowserWidget::ImportDirectoryMode’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 373 | importDirectoryModeComboBox->addItem(ctkDICOMVisualBrowserWidget::tr("Add Link"), ctkDICOMVisualBrowserWidget::ImportDirectoryAddLink); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:373:136: warning: in call to ‘QVariant::QVariant(int)’ [-Wsign-promo] /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:374:129: warning: passing ‘ctkDICOMVisualBrowserWidget::ImportDirectoryMode’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 374 | importDirectoryModeComboBox->addItem(ctkDICOMVisualBrowserWidget::tr("Copy"), ctkDICOMVisualBrowserWidget::ImportDirectoryCopy); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:374:129: warning: in call to ‘QVariant::QVariant(int)’ [-Wsign-promo] /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:385:71: warning: passing ‘ctkDICOMVisualBrowserWidget::ImportDirectoryMode’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 385 | importDirectoryModeComboBox->findData(q->importDirectoryMode())); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:385:71: warning: in call to ‘QVariant::QVariant(int)’ [-Wsign-promo] [ 89%] Building CXX object Libs/DICOM/Widgets/CMakeFiles/CTKDICOMWidgets.dir/moc_ctkDICOMPatientItemWidget.cpp.o /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp: In member function ‘void ctkDICOMVisualBrowserWidget::setImportDirectoryMode(ctkDICOMVisualBrowserWidget::ImportDirectoryMode)’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:1950:52: warning: passing ‘ctkDICOMVisualBrowserWidget::ImportDirectoryMode’ chooses ‘int’ over ‘uint’ {aka ‘unsigned int’} [-Wsign-promo] 1950 | comboBox->setCurrentIndex(comboBox->findData(mode)); | ^ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:1950:52: warning: in call to ‘QVariant::QVariant(int)’ [-Wsign-promo] /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp: In member function ‘void ctkDICOMVisualBrowserWidget::setDatabaseDirectory(const QString&)’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:1995:27: warning: catching polymorphic type ‘class std::exception’ by value [-Wcatch-value=] 1995 | catch (std::exception e) | ^ [ 92%] Building CXX object Libs/DICOM/Widgets/CMakeFiles/CTKDICOMWidgets.dir/moc_ctkDICOMSeriesItemWidget.cpp.o /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp: In member function ‘void ctkDICOMVisualBrowserWidget::exportSelectedItems(ctkDICOMModel::IndexType, QList)’: /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:3034:45: warning: ‘QFileDialog::DirectoryOnly’ is deprecated: Use setOption(ShowDirsOnly, true) instead [-Wdeprecated-declarations] 3034 | directoryDialog->setFileMode(QFileDialog::DirectoryOnly); | ^~~~~~~~~~~~~ In file included from /home/jcfr/Support/Qt/5.15.2/gcc_64/include/QtWidgets/QFileDialog:1, from /path/to/CTK/Libs/Widgets/ctkDirectoryButton.h:26, from /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:39: /home/jcfr/Support/Qt/5.15.2/gcc_64/include/QtWidgets/qfiledialog.h:84:21: note: declared here 84 | DirectoryOnly Q_DECL_ENUMERATOR_DEPRECATED_X("Use setOption(ShowDirsOnly, true) instead")}; | ^~~~~~~~~~~~~ /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:3034:45: warning: ‘QFileDialog::DirectoryOnly’ is deprecated: Use setOption(ShowDirsOnly, true) instead [-Wdeprecated-declarations] 3034 | directoryDialog->setFileMode(QFileDialog::DirectoryOnly); | ^~~~~~~~~~~~~ In file included from /home/jcfr/Support/Qt/5.15.2/gcc_64/include/QtWidgets/QFileDialog:1, from /path/to/CTK/Libs/Widgets/ctkDirectoryButton.h:26, from /path/to/CTK/Libs/DICOM/Widgets/ctkDICOMVisualBrowserWidget.cpp:39: /home/jcfr/Support/Qt/5.15.2/gcc_64/include/QtWidgets/qfiledialog.h:84:21: note: declared here 84 | DirectoryOnly Q_DECL_ENUMERATOR_DEPRECATED_X("Use setOption(ShowDirsOnly, true) instead")}; | ^~~~~~~~~~~~~ ```
Punzo commented 5 months ago

@jcfr thanks for all the rebasing and fixes!!!! just few notes for the PR messagges:

1) this is the updated UML diagram: image 2) and here the updated scripts:

import os

server = ctk.ctkDICOMServer()
server.connectionName = "ConnectionName"
server.callingAETitle = "callingAETitle"
server.calledAETitle = "calledAETitle"
server.host = "host"
server.port = 11112
server.retrieveProtocol = ctk.ctkDICOMServer.CGET

DICOMDatabase = qt.QSettings().value(slicer.dicomDatabaseDirectorySettingsKey)
if not os.path.isdir(DICOMDatabase):
  os.makedirs(DICOMDatabase)  

browser = ctk.ctkDICOMVisualBrowserWidget()
browser.addServer(server)
browser.setDatabaseDirectory(DICOMDatabase)
browser.filteringPatientID = 1
browser.resize(slicer.util.mainWindow().size)
browser.show()
browser.onQueryPatients()

server = ctk.ctkDICOMServer()
server.connectionName = "ConnectionName"
server.callingAETitle = "callingAETitle"
server.calledAETitle = "calledAETitle"
server.host = "host"
server.port = 11112
server.retrieveProtocol = ctk.ctkDICOMServer.CGET

scheduler = ctk.ctkDICOMScheduler()
scheduler.setDicomDatabase(slicer.dicomDatabase)
scheduler.addServer(server)

nDays = 30
endDate = qt.QDate().currentDate()
startDate = endDate.addDays(-nDays);
parameters = {
  "ID": "CT",
  #"Name": "name",
  #"Study": "description",
  #"Series": "description",
  #"Modalities": ["CT", "MR"],
  #"StartDate": startDate.toString("yyyyMMdd"),
  #"EndDate": endDate.toString("yyyyMMdd")
}

scheduler.setFilters(parameters)
# Use one of the following methods: all these methods run background processes!!!!
#scheduler.queryPatients()
#scheduler.queryStudies('patientID')
#scheduler.querySeries('patientID', 'studyInstanceUID')
#scheduler.queryInstances('patientID', 'studyInstanceUID', 'seriesInstanceUID')
#scheduler.retrieveStudy('patientID', 'studyInstanceUID')
#scheduler.retrieveSeries('patientID','studyInstanceUID', 'seriesInstanceUID')
#scheduler.retrieveSOPInstance('patientID', 'studyInstanceUID', 'seriesInstanceUID', 'SOPInstanceUID')
jcfr commented 5 months ago

Some of the tests are failing ... it would be great to fix these as well:

$ ctest -R ctkDI
Test project /home/jcfr/Projects/CTK-Qt5-build/CTK-build
      Start 182: ctkDICOMCoreTest1
 1/47 Test #182: ctkDICOMCoreTest1 ....................   Passed    0.04 sec

[...]

      Start 315: ctkDICOMApplicationTest1
47/47 Test #315: ctkDICOMApplicationTest1 .............   Passed    0.23 sec

81% tests passed, 9 tests failed out of 47

[...]

Total Test time (real) =  32.83 sec

The following tests FAILED:
    195 - ctkDICOMEchoTest1 (Failed)
    199 - ctkDICOMQueryTest2 (Failed)
    201 - ctkDICOMRetrieveTest2 (Failed)
    202 - ctkDICOMSchedulerTest1 (Failed)
    203 - ctkDICOMTesterTest1 (Failed)
    204 - ctkDICOMTesterTest2 (Failed)
    216 - ctkDICOMAppWidgetTest1 (SEGFAULT)
    217 - ctkDICOMBrowserTest (Subprocess aborted)
    305 - ctkDICOMHostTest1 (Failed)
Punzo commented 5 months ago

Some of the tests are failing ... it would be great to fix these as well:

$ ctest -R ctkDI
Test project /home/jcfr/Projects/CTK-Qt5-build/CTK-build
      Start 182: ctkDICOMCoreTest1
 1/47 Test #182: ctkDICOMCoreTest1 ....................   Passed    0.04 sec

[...]

      Start 315: ctkDICOMApplicationTest1
47/47 Test #315: ctkDICOMApplicationTest1 .............   Passed    0.23 sec

81% tests passed, 9 tests failed out of 47

[...]

Total Test time (real) =  32.83 sec

The following tests FAILED:
  195 - ctkDICOMEchoTest1 (Failed)
  199 - ctkDICOMQueryTest2 (Failed)
  201 - ctkDICOMRetrieveTest2 (Failed)
  202 - ctkDICOMSchedulerTest1 (Failed)
  203 - ctkDICOMTesterTest1 (Failed)
  204 - ctkDICOMTesterTest2 (Failed)
  216 - ctkDICOMAppWidgetTest1 (SEGFAULT)
  217 - ctkDICOMBrowserTest (Subprocess aborted)
  305 - ctkDICOMHostTest1 (Failed)

ok I am running them and double-checking.

It may also be related to the second part of https://github.com/commontk/CTK/pull/1142#issuecomment-1856174973. Reporting here:

Note/question for @jcfr: To run the test locally, I had to follow these steps:

CTK_BUILD_ALL                    OFF
CTK_BUILD_ALL_APPS               OFF
CTK_BUILD_ALL_LIBRARIES          OFF
CTK_BUILD_ALL_PLUGINS            OFF
CTK_BUILD_EXAMPLES               ON
CTK_ENABLE_DICOM                 ON
CTK_ENABLE_DICOMApplicationHos   OFF
CTK_ENABLE_PluginFramework       OFF
CTK_ENABLE_Python_Wrapping       OFF
CTK_ENABLE_Widgets               ON

I might be missing something, but it would be helpful if the superbuild could automatically install DCMTK (eliminating the need for steps 3 and 4). Please let me know if this is a bug or if I am overlooking something.

Punzo commented 5 months ago

@jcfr I confirm with the ld library path fixed manually, only these tests fails:

he following tests FAILED: 167 - ctkDICOMSchedulerTest1 (Failed) 181 - ctkDICOMAppWidgetTest1 (SEGFAULT) 182 - ctkDICOMBrowserTest (Subprocess aborted)

I am going to fix them now.

If you can have a look how to fix the installation of DCMTK, it would be great (I don't want to mess up the cmake for projects relying on CTK).

jcfr commented 5 months ago

re: LD_LIBRARY_PATH + lookup binaries

Thanks for the details. I will look into fixing the build-system to automate this.

re: UML & testing scripts

I updated the main description based of https://github.com/commontk/CTK/pull/1165#issuecomment-1888809913 to include the updated diagram & testing scripts

Punzo commented 5 months ago

@jcfr tests fixed, I could not push on your branch, I opened this PR https://github.com/jcfr/CTK/pull/1

jcfr commented 5 months ago

Thanks @Punzo :pray: , your changes were helpful to move forward.

Fixes and improvement to the DICOM tests are being integrated in the following pull requests:

Once there are integrated, we will rebase this one and ensure all tests are still passing :100: :rocket:

Thanks for your patience :hourglass_flowing_sand:

lassoan commented 5 months ago

Thank you @jcfr for workong on this a lot! What is the status of this now? Can I help with anything to get this integrated?

jcfr commented 5 months ago

Proposed plan:

lassoan commented 5 months ago

Sounds good, thank you!

Punzo commented 5 months ago

@jcfr can you please add this commit https://github.com/jcfr/CTK/pull/2 before merging? thanks in advance!

jcfr commented 5 months ago

The API has been cleanup, @Punzo could you also test on your side ?

Summary:

jcfr commented 5 months ago

@Punzo What do you think of updating one of the test by adding a QSignalSpy and checking that these new signals are effectively invoked when removing data ?

Punzo commented 5 months ago

@jcfr thanks for all the cleaning! I will test right now.

@Punzo What do you think of updating one of the test by adding a QSignalSpy and checking that these new signals are effectively invoked when removing data ?

ok, I will have a look.

Punzo commented 5 months ago

@jcfr thanks for all the cleaning! I will test right now.

@jcfr found a small bug while testing the ctkDICOMVisualBrowser binaries (the GUI was not updating), see PR at https://github.com/jcfr/CTK/pull/3

testing now the PR with Slicer.

Punzo commented 5 months ago

testing now the PR with Slicer.

Done, everything working (with the small patch https://github.com/jcfr/CTK/pull/3)!

Punzo commented 5 months ago

@Punzo What do you think of updating one of the test by adding a QSignalSpy and checking that these new signals are effectively invoked when removing data ?

ok, I will have a look.

Done, see https://github.com/jcfr/CTK/pull/3/commits/923eaeb282112b7410a0cdeb4a17c690597f6ded

jcfr commented 5 months ago

I am in the process of finalizing the integration of some dependent DICOM tests fix, I will then rebase this PR and integrate it as well.

Thanks for the patience

Punzo commented 5 months ago

@jcfr would be possible to merge this, please?

I am currently working on the job list UI (https://github.com/commontk/CTK/pull/1184) and I would like to avoid conflicts and multiple rebasing.

If you think further cleaning is necessary, no problem. I will rebase 1184 once you merge into master.

lassoan commented 5 months ago

Are the commits going to be squashed? I don't think we need to separate commits for adding new code and then for making fixes or enhancements on that new code - it could be all just one commit to keep the version history less noisy.

Punzo commented 5 months ago

Are the commits going to be squashed? I don't think we need to separate commits for adding new code and then for making fixes or enhancements on that new code - it could be all just one commit to keep the version history less noisy.

I think Jc was planning to squash before mergning with this commit message https://github.com/commontk/CTK/pull/1165#issuecomment-1888661303

jcfr commented 5 months ago

I am finalizing the cleanups and Will get this integrated this afternoon.

Punzo commented 5 months ago

I am finalizing the cleanups and Will get this integrated this afternoon.

ok thanks a lot!

jcfr commented 5 months ago

@Punzo Could you perform a last round of test ?

Punzo commented 5 months ago

@Punzo Could you perform a last round of test ?

sure! done and everything looks good. Thanks a lot for your hard work on the cleaning, I appreciate it.

  • Consider reviewing the commit Simplify copy of JobResponseSet introducing clone() (one of the most recent)

Checked, looks good to me. I have also tested with valgrind/massif and the memory usage/deallocation works as expected.

  • Note that ctkAbstractScheduler was renamed to ctkJobScheduler as it is not abstract anymore. You may want to update the diagram
  • While the class ctkDICOMWorker but is not fulfilling any purpose, we could keep it around ...

I agree, I removed it in https://github.com/commontk/CTK/pull/1184/commits/6afcfe5cb9772c3f950bf704334d7f0558d11ad6

and here the new UML

ctkVisualDICOMBrowser

ah that's great!!!

lassoan commented 5 months ago

🍾 Amazing job guys, really looking forward to seeing this working in Slicer!