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

feat: unify data IDs #594

Merged
merged 2 commits into from
May 21, 2024
Merged

feat: unify data IDs #594

merged 2 commits into from
May 21, 2024

Conversation

floryst
Copy link
Collaborator

@floryst floryst commented May 20, 2024

Volume keys are now used as image data IDs. This simplifies going back and forth between the image and dicom store, and simplifies the structure of the selection object to be just an ID string.

This doesn't address migration of older session files. That is a nice-to-have, but we don't have a good way of handling migrations at the moment. @PaulHax if this breaks stored session files, then I'll add a migration path.

@floryst floryst requested a review from PaulHax May 20, 2024 17:22
Copy link

netlify bot commented May 20, 2024

Deploy Preview for volview-dev ready!

Name Link
🔨 Latest commit e4da486
🔍 Latest deploy log https://app.netlify.com/sites/volview-dev/deploys/664cb7b38aacdb0007a5bda2
😎 Deploy Preview https://deploy-preview-594--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@PaulHax PaulHax left a comment

Choose a reason for hiding this comment

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

So nice. Does break loading old session files with layers due to the dropping of the gross image/dicom prefix on the layer IDs. But I'm OK with that.

old-layers.volview.zip

src/store/datasets-dicom.ts Outdated Show resolved Hide resolved
Volume keys are now used as image data IDs. This simplifies going back
and forth between the image and dicom store, and simplifies the
structure of the selection object to be just an ID string.
@floryst
Copy link
Collaborator Author

floryst commented May 21, 2024

So nice. Does break loading old session files with layers due to the dropping of the gross image/dicom prefix on the layer IDs. But I'm OK with that.

We can figure out a migration layer later if this becomes a major issue. For now, I've bumped the manifest major version.

@PaulHax
Copy link
Collaborator

PaulHax commented May 21, 2024

When/if we get a complaint, we have some migrate code here:

const migrateOrPass =
(versions: Array<string>, migrationFunc: (manifest: any) => any) =>
(inputManifest: any) => {
if (versions.includes(inputManifest.version)) {
return migrationFunc(inputManifest);
}
return inputManifest;
};
const migrateBefore210 = (inputManifest: any) => {
const manifest = JSON.parse(JSON.stringify(inputManifest));
manifest.version = '2.1.0';
return manifest;
};
const migrate210To300 = (inputManifest: any) => {
const manifest = JSON.parse(JSON.stringify(inputManifest));
manifest.tools.paint.activeSegmentGroupID =
inputManifest.tools.paint.activeLabelmapID;
delete manifest.tools.paint.activeLabelmapID;
const order = Object.keys(LABELMAP_PALETTE_2_1_0).map((key) => Number(key));
manifest.labelMaps = inputManifest.labelMaps.map(
(labelMap: any, index: number) => ({
id: labelMap.id,
path: labelMap.path,
metadata: {
parentImage: labelMap.parent,
name: makeDefaultSegmentGroupName('My Image', index),
segments: {
order,
byValue: LABELMAP_PALETTE_2_1_0,
},
},
})
);
manifest.version = '3.0.0';
return manifest;
};
const migrateManifest = (manifestString: string) => {
const inputManifest = JSON.parse(manifestString);
return pipe(
inputManifest,
migrateOrPass(['1.1.0', '1.0.0', '0.5.0'], migrateBefore210),
migrateOrPass(['2.1.0'], migrate210To300)
);
};

@floryst floryst added this pull request to the merge queue May 21, 2024
Merged via the queue into main with commit 9e650be May 21, 2024
7 checks passed
1isten added a commit to 1isten/VolView that referenced this pull request Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants