Davide_Punzo:
@Davide_Punzo it looks like the Count column is empty after the update. Can you test if this is a side-effect of updating the database or is it something that happens if you start with a fresh database (i.e. is the count populated in a new database).
it is a general bug. I have investigated it and the issue is that:
in dcdeftag.h we have this definition: #define DCM_SeriesInstanceUID DcmTagKey(0x0020, 0x000e)
While in the metadata of that specific DICOM in my video the seriesIstanceUID tag is "0020,000E"
this create the issue in ctkDICOMDisplayedFieldGeneratorSeriesImageCountRule::getDisplayedFieldsForInstance:CTK/Libs/DICOM/Core/ctkDICOMDisplayedFieldGeneratorSeriesImageCountRule.cpp at 7ed1da357b9e7e2462b4b764882612484c6fa051 · commontk/CTK · GitHub
because the return of cachedTagsForInstance[dicomTagToString(DCM_SeriesInstanceUID)] is an empty string (because dicomTagToString(DCM_SeriesInstanceUID) return “0020,000e” instead of “0020,000E”) and this creates issues when updating the tables.
I am not sure if this is a new regression, but I will check out differences in the CTK commits after Slicer version 5.6.2 and see if I can find where the regression happened.
@pieper @cpinter not sure what is the best way to fix the issue. Maybe we should fix the dicoms at loading/fetching time to have lowercase in the tags, instead of putting a lot of if exceptions everytime we use dicomTagToString to check lowercase vs uppercase
@pieper I think the issue is this commit:
committed 03:57PM - 13 Dec 23 UTC
This generalizes the ctkDICOMDatabase code to ease hard-coded restrictions about…
DICOM files always being on disk. Now the schema includes a `URL` field of the
SQLite database so certain file-specific operations are only performed on filePaths
while URL are handled independently. Entries in the `Images` table of the database
may now have either a `URL` or a `fileName` or both and new accessors are provided
to get `URL`s at the series and instance level.
Schema version is updated from version `0.7.0` to `0.8.0`.
Also this contains a fix to the `ctkDICOMTagCache` so that tags are always stored
internally as uppercase hex, so a string like "0010,000d" will always be turned
into "0010,000D" for storage in the database. Both forms can be used to query
tags. This avoids the situation where some cache entries were duplicated because
different code used either upper or lower case to refer to the same tag. Using
only upper case should improve storage use and improve performance by avoiding
unneeded cache misses.
Co-authored-by: Andras Lasso <lasso@queensu.ca>
Co-authored-by: Davide Punzo <punzodavide@hotmail.it>
Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
I am not sure 100%, it could be a mix of things, but my investigation points to that. This because the count display works only up to Slicer version 5.6.2 which uses the dicom schema 0.7.0, i.e just before the linked commit which modifies the lower/upper case tags stuff too.
Adding .toUpper() in CTK/Libs/DICOM/Core/ctkDICOMDisplayedFieldGeneratorAbstractRule.h at 7ed1da357b9e7e2462b4b764882612484c6fa051 · commontk/CTK · GitHub :
/// Utility function to convert a DICOM tag enum to string
static QString dicomTagToString(const DcmTagKey& tag)
{
return QString("%1,%2").arg(tag.getGroup(),4,16,QLatin1Char('0')).arg(tag.getElement(),4,16,QLatin1Char('0')).toUpper();
}
fix the issue, I have created a PR in CTK (BUG: Convert lower case dicom tags by Punzo · Pull Request #1203 · commontk/CTK · GitHub ). But I am not sure if we need to apply the fix in any another part of CTK
1 Like