Fixes #236 / #118 possibility to add filters on selected volumes folder #252

AcastaPaloma · 2025-06-22T17:35:57Z

Previous behavior
Upon selecting a volumes folder, it was impossible to filter/select specific cases (case paths) based off of characteristics in the file name.

Current behavior
A user can add filters of inclusion and exclusion. Loaded files from a selected volume are files that all have all inclusion filters within their file name and none of the exclusion filters within their file name. Casing does not matter (t1 == T1)

Some light UI changes included in this PR, could be applied to all windows:

Adding fields to a window no longer requires a new window => Could reduce the amount of windows whenever something new is added (such as fields)
Some spacing between configuration fields for readability

Currently, limited testing has been done, this PR is for reviews and a showcase of the UI
Action steps:

Adequate testing of existing functionalities
Testing on Continue from existing segmentation (filtering should not be an option)
Adding a button to modify filters, thus changing the active case list during a single segmentation session, if session is a New Configuration type

…://github.com/neuropoly/slicercart into kyw/issue213-remove-edit-configuration-button

…exclusion and inclusion filters

…ve the include

AcastaPaloma · 2025-06-22T17:38:23Z

@maxradx @SomeoneInParticular I only tested it on configuration_config.yaml
Just first wanted your opinions on the UI and the existing functions.

SomeoneInParticular · 2025-06-22T22:05:56Z

Will do @AcastaPaloma; for future reference, if you want me (or anyone else) to review something, assign us as a reviewer as well. It gives us a notification that is much clearer than "someone somewhere mentioned you", and in some repositories is required for us to initiate a review of the code base/provide suggestions in the first place.

…sion filters and not all

AcastaPaloma · 2025-06-23T16:13:14Z

Got it @SomeoneInParticular

There is now a known bug: upon selecting the output folder, filters' effect seem to be nulled. Like mentioned, will tackle Continue from existing segmentation configuration

SomeoneInParticular

Excellent work! I have a few remarks that should be addressed before we merge, but overall this is an excellent addition.

SomeoneInParticular · 2025-06-23T19:59:40Z

SlicerCART/src/SlicerCART.py

+        if self.config_yaml["case_list_filters"]["exclusion"]:
+            filters_to_exclude = self.config_yaml["case_list_filters"]["exclusion"]
+        else:
+            filters_to_exclude = []


These are dict-like objects, which means we can use Python's get function w/ a default return to make them cleaner (and faster). I also replaced the empty list (which you end up replacing with None in the next function anyways) with None:

filters_to_include = self.config_yaml["case_list_filters"].get("inclusion", []) filters_to_exclude = self.config_yaml["case_list_filters"].get("exclusion", [])

SomeoneInParticular · 2025-06-23T20:02:47Z

SlicerCART/src/SlicerCART.py

+            if inclusion == []:
+                inclusion = None
+            if exclusion == []:
+                exclusion = None


Remove this; its entirely unnecessary given inclusion/exclusion are always a list (see prior remark), and thus the following list comprehension will always work with my suggested change.

SomeoneInParticular · 2025-06-23T20:03:38Z

SlicerCART/src/SlicerCART.py

+            inclusion = [item.lower() for item in (inclusion or [])]
+            exclusion = [item.lower() for item in (exclusion or [])]


Suggested change

inclusion = [item.lower() for item in (inclusion or [])]

exclusion = [item.lower() for item in (exclusion or [])]

inclusion = [item.lower() for item in inclusion]

exclusion = [item.lower() for item in exclusion]

You pseudo-cast the list to None, then immediately replace it with an empty list again here. Seems odd, and may potentially mislead future developers.

SomeoneInParticular · 2025-06-23T20:05:56Z

SlicerCART/src/configuration_config.yml

 default_volume_directory: ''
 enable_debug: true
 freetextboxes:
-  cheese: Cheese


Thank you; I was wondering what this was doing here...

SomeoneInParticular · 2025-06-23T20:24:30Z

SlicerCART/src/configuration_config.yml

-slice_view_color: Yellow
+slice_view_color: Red


Is there a reason for this change? Not that is particularly matters, but I'm curious...

I realized that oftentimes, MRIs or CTs are taken on the axial; loading them on the sagittal/coronal would just result in a distorted image and could confuse first time users.

Valid; how does that relate to the color though? Or is this some poorly documented Slicer specific requirement?

SomeoneInParticular · 2025-06-23T21:25:29Z

SlicerCART/src/scripts/InteractingClasses.py

            # msg.buttonClicked.connect(self.push_error_label_list_empty)
            msg.exec()
        else:
-            ConfigPath.write_config_file()


Why was this removed? For debugging, or was it causing an issue/conflict?

SomeoneInParticular · 2025-06-23T23:26:50Z

SlicerCART/src/scripts/InteractingClasses.py

+        # Check for duplicates within each category
+        def find_duplicates(lst):
+            return set(x for x in lst if lst.count(x) > 1)


Why don't we handle this at entry, rather than at application?

SomeoneInParticular · 2025-06-24T00:12:12Z

SlicerCART/src/scripts/InteractingClasses.py

+        # Check for empty filters
+        if "" in inclusion or "" in exclusion:
+            qt.QMessageBox.warning(self, "Warning", "All filters must be filled. Please remove or fill empty filters.")
+            return


Why not just purge them in the first place then?

SomeoneInParticular · 2025-06-24T00:14:36Z

SlicerCART/src/scripts/InteractingClasses.py

+            self.inclusion_table_view.setRowCount(num_inclusion_filters)
+
+            for incl_idx, incl_filter_value in enumerate(self.case_list_filters["inclusion"]):
+                # Create the remove filter button


Please go through the code and add comments like this for all buttons/UI elements. For some of the more common functions, you might even want to refactor them to standalone functions for modularity and reuse.

SomeoneInParticular · 2025-06-24T00:31:35Z

A few other concerns as well:

The changes in this PR seem to have broken the initial rendering of the MRI sequences when an input folder is selected. Once and output folder is selected, it works fine, but something is wrong here and needs to be addressed...
Removing an inclusion/exclusion filter forces the entire window prompt to close and re-open; adding one doesn't, though. If possible, try to make the former act like the latter (so it doesn't "jump" every time an entry is removed).

AcastaPaloma · 2025-06-24T15:22:33Z

for myself

AcastaPaloma added 18 commits June 17, 2025 13:33

removed cheese

d6a5818

Merge branch 'kyw/issue213-remove-edit-configuration-button' of https…

875e9f3

…://github.com/neuropoly/slicercart into kyw/issue213-remove-edit-configuration-button

moved select output folder closer to select volumes folder

852c30d

UI done for first window displaying all filters with add buttons for …

57c973f

…exclusion and inclusion filters

backup: UI changes

48fb6c2

Merge branch 'main' into kyw/118-filter-case-list

3157dfb

UI done for add inclusion filters

740ca70

add inclusion and add exclusion both work, missing connect with yaml

0d61dc3

backup

471bf01

backup

4b8974c

removal on inlusion filters works

569ab95

working interface, with saving

36a0eea

filters work, before saving and loading appropriate files

70d9eb0

half working sorting, inclusion problematic

2c1713c

working filtering

fb47dcf

clean

c0964bb

needs to tackle those who do not have the exclude, but also do not ha…

9cbec9b

…ve the include

working filters and working loading of filtered case list

081cd30

SomeoneInParticular self-requested a review June 23, 2025 04:13

AcastaPaloma added 3 commits June 23, 2025 10:18

temp

16bd73c

removal of temp video links to project week 43

20e25c2

fixes bug: selected paths in case list now have at least one of inclu…

74f74fa

…sion filters and not all

SomeoneInParticular requested changes Jun 24, 2025

View reviewed changes

revert of accidental removal

3244666

CART LINK

bb65219

AcastaPaloma marked this pull request as draft July 7, 2025 16:12

AcastaPaloma changed the title ~~Fixes #236 / #118 possibility to add filters on selected volums folder~~ Fixes #236 / #118 possibility to add filters on selected volumes folder Aug 8, 2025

		inclusion = [item.lower() for item in (inclusion or [])]
		exclusion = [item.lower() for item in (exclusion or [])]

		slice_view_color: Yellow
		slice_view_color: Red

Fixes #236 / #118 possibility to add filters on selected volumes folder #252

Are you sure you want to change the base?

Fixes #236 / #118 possibility to add filters on selected volumes folder #252

Uh oh!

Conversation

AcastaPaloma commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcastaPaloma commented Jun 22, 2025

Uh oh!

SomeoneInParticular commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcastaPaloma commented Jun 23, 2025

Uh oh!

SomeoneInParticular left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SomeoneInParticular Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SomeoneInParticular commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcastaPaloma commented Jun 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AcastaPaloma commented Jun 22, 2025 •

edited

Loading

SomeoneInParticular commented Jun 22, 2025 •

edited

Loading

SomeoneInParticular Jun 24, 2025 •

edited

Loading

SomeoneInParticular commented Jun 24, 2025 •

edited

Loading