-
Notifications
You must be signed in to change notification settings - Fork 65
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
Segment group file format selection for serialization #527
Conversation
✅ Deploy Preview for volview-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
6286cb4
to
97257f2
Compare
97257f2
to
e2a0984
Compare
e2a0984
to
df5e8f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if there's going to be a conflict in the future regarding the segmentation information that is stored in the session's json file vs the embedded info stored inside .nrrd and other fancier file formats that we might support in the future.
4345276
to
a2634a7
Compare
This is good to go by me. Followup PR for GUI export button. |
0df0301
to
94693c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, apart from a few general comments.
import { FileReaderMap } from '.'; | ||
|
||
import { readFileAsArrayBuffer } from './io'; | ||
import { stlReader, vtiReader, vtpReader } from './vtk/async'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like itk-wasm/mesh-io
also has a similar generic reader for polydata? Maybe in the future we could get rid of these special readers as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, cool. VolView's mesh support is just vestigial at the moment.
@@ -203,6 +203,7 @@ export const useDicomWebStore = defineStore('dicom-web', () => { | |||
state: 'Done', | |||
}; | |||
} catch (error) { | |||
console.error(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to keep this, or was just added during debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good check, did want to keep actualy.
Add segmentGroupSaveFormat section.
55cb8a3
to
67d2631
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Haven't tested functionality yet.
VolView will save "segment group" images into a file format configurable by a config.JSON file.
deserialize
will read whatever format it finds in the .zip file.This is the config json to change the saved format
To test: Can drag and drop this config file on VolView before saving.
config-save-format.json
Possible formats here:
https://github.com/InsightSoftwareConsortium/itk-wasm/blob/main/packages/image-io/typescript/src/extension-to-image-io.ts
TODO
- [ ] Add GUI to set format, maybeFollow up PR to add GUI.