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.

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

Wrong storage-sop-classes.properties UID Renaming #843

Closed
NicolasLaheurte opened this issue Dec 2, 2020 · 7 comments
Closed

Wrong storage-sop-classes.properties UID Renaming #843

NicolasLaheurte opened this issue Dec 2, 2020 · 7 comments
Assignees
Labels
Milestone

Comments

@NicolasLaheurte
Copy link

NicolasLaheurte commented Dec 2, 2020

In dcm4che-assembly/src/etc/dcmqrscp/storage-sop-classes.properties it seems the keyword "Retired" has been removed also when unnecessary.

Seen issues:

  1. UltrasoundMultiFrameImageStorageRetired
    UltrasoundMultiFrameImageStorage
    Both renamed to:
    UltrasoundMultiFrameImageStorage

  2. UltrasoundImageStorageRetired
    UltrasoundImageStorage
    Both renamed to:
    UltrasoundImageStorage

  3. "NuclearMedicineImageStorageRetired" became "NuclearMedicineImageStorage" which isn't the same UID (both are existing).

No issue seen in query-sop-classes.properties / retrieve-sop-classes.properties but the same issue is found in sop-classes.properties.

@gunterze gunterze self-assigned this Dec 2, 2020
@gunterze gunterze added the bug label Dec 2, 2020
@gunterze gunterze added this to the 5.23.0 milestone Dec 2, 2020
@jssuttles
Copy link

Hey,
This fix wasn't quite right.
NuclearMedicineImageStorage is different from NuclearMedicineImageStorageRetired.
1.2.840.10008.5.1.4.1.1.20
vs
1.2.840.10008.5.1.4.1.1.5

@gunterze
Copy link
Member

Hey, This fix wasn't quite right. NuclearMedicineImageStorage is different from NuclearMedicineImageStorageRetired. 1.2.840.10008.5.1.4.1.1.20 vs 1.2.840.10008.5.1.4.1.1.5

Fixed files contain

NuclearMedicineImageStorage:*

and

NuclearMedicineImageStorageRetired:*

!

@jssuttles
Copy link

Hunh. Sorry about that. You're right. Somehow I missed that in my search. Thanks!

@jssuttles
Copy link

Hi, sorry for bothering you again.
But I just checked through https://github.com/dcm4che/dcm4che/blob/master/dcm4che-assembly/src/etc/storescp/sop-classes.properties again and it's not there.
It was removed in ba4626d#diff-777459a77c74a7f0837eb94f2a5a7141f4d8a8269affcb4dc2ef263e225b5eca.
Does this commit make sense?
image

@gunterze
Copy link
Member

Also don't see the intention of ba4626d
Will ask @nroduit .

@nroduit
Copy link
Member

nroduit commented Oct 14, 2021

The intention was only to remove duplicates. However, I don't see any duplicates now!

@gunterze
Copy link
Member

gunterze commented Oct 14, 2021

Duplicates were already removed by 738437d , so when ba4626d removed NuclearMedicineImageStorage:*, only NuclearMedicineImageStorageRetired:* remained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants