Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove filtered displaySets from parameter from study protocolEngine.findMatch() #3343

Merged
merged 5 commits into from
Jun 29, 2023
Merged

Remove filtered displaySets from parameter from study protocolEngine.findMatch() #3343

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d65ebb2
Remove filtered displaySets from parameter from study protocolEngine.…
Sofien-Sellami Apr 28, 2023
9b3047d
Merge branch 'v3-stable' into Remove_filtred_studyDisplaySets_parameter
Sofien-Sellami May 25, 2023
bb07063
adding studyDisplaySets
Sofien-Sellami May 26, 2023
53f51cf
Refact
Sofien-Sellami Jun 2, 2023
9d27066
Merge branch 'master' into Remove_filtred_studyDisplaySets_parameter
wayfarer3130 Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions platform/core/src/services/HangingProtocolService/HangingProtocolService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,15 +1303,13 @@ export default class HangingProtocolService extends PubSubService {
this.studies.forEach(study => {
// Skip non-active if active only
if (matchActiveOnly && this.activeStudy !== study) return;
const studyDisplaySets = this.displaySets.filter(
it => it.StudyInstanceUID === study.StudyInstanceUID
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't move this change, but leave it here, then you can pass both displaySets and studyDisplaySets into the findMatch. Then, custom attributes can be setup to choose either of them - and it is only custom attributes which access these.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking it over, probably leaving displaySets as is, but passing allDisplaySets into the findMatch function would be more consistent. The rule is testing this study right now, so it should have the display sets for this study, not all display sets, and if a particular rule wants to look at all display sets, then just having it as a another value would make that possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, at the study matching we need to access the list of registered study and display sets.
For unknown reason, this.studies is not updated when you add available studies in OHIF (for instance by opening priors panel), but displayset are correctly added.
At the study matching you need to access all studies and all display set (listing only displayset of the current study will forbid cross study matching for which study matching rule stand for).

So sofien updated his code to have :
displayset === all known displayset at the study level
displayset === studies display set at the series level

const studyMatchDetails = this.protocolEngine.findMatch(
study,
studyMatchingRules,
{
studies: this.studies,
displaySets: studyDisplaySets,
displaySets: this.displaySets,
wayfarer3130 marked this conversation as resolved.
Show resolved Hide resolved
displaySetMatchDetails: this.displaySetMatchDetails,
}
);
Expand All @@ -1321,6 +1319,9 @@ export default class HangingProtocolService extends PubSubService {
return;
}

const studyDisplaySets = this.displaySets.filter(
it => it.StudyInstanceUID === study.StudyInstanceUID
);
this.debug(
'study',
study.StudyInstanceUID,
Expand Down