-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add webknossos_annotation.py #11
Add webknossos_annotation.py #11
Conversation
Co-authored-by: Kabilar Gunalan <[email protected]>
Co-authored-by: Kabilar Gunalan <[email protected]>
Co-authored-by: Kabilar Gunalan <[email protected]>
Co-authored-by: Kabilar Gunalan <[email protected]>
max_load: int = 16384, | ||
nii: bool = False, | ||
has_channel: int = 1, | ||
): |
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.
Can you document what each input/option does?
scripts/webknossos_annotation.py
Outdated
low_res_offsets.append([t,b,l,r]) | ||
|
||
# load jp2 image to get voxel size info | ||
j2k = glymur.Jp2k(jp2_dir) |
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.
It might be better to reference the zarr file that was annotated. It should have the voxel size info as well.
scripts/webknossos_annotation.py
Outdated
|
||
dic_EB = {0:0, 1:1, 9:2, 2:3, 4:4, 10:5, 11:6, 8:7, 3:8} | ||
dic_JS = {0:0, 4:2, 5:3, 9:4, 6:5, 2:6, 7:7, 3:8} | ||
dic_JW = {0:0, 1:1, 2:2, 3:3, 4:4, 6:5, 7:6, 8:7, 9:8} |
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.
Can you add a comment to describe what these dictionaries contain?
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.
Also, should this be an input option somehow?
|
||
|
||
nblevel = 9 | ||
# Write each level |
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.
we should probably get the number of levels from the data, not hardcode it.
scripts/webknossos_annotation.py
Outdated
for level in range(nblevel): | ||
omz_res = omz_data[level] | ||
size = omz_res.shape[-2:] | ||
shape = [1, 20] + [i for i in size] |
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.
Same comment - do we need to harcode this?
|
||
|
||
# Write OME-Zarr multiscale metadata | ||
print('Write metadata') |
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 the annotation have the exact same shape as the annotated zarr file? I feel like we should just copy that info from the input zarr.
scripts/webknossos_annotation.py
Outdated
|
||
|
||
def generate_mask(mask): | ||
final_mask = np.zeros_like(mask).astype(np.uint8) |
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.
What is this doing? Is it a good thing to process the data in a "conversion" script?
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.
Hi @jingjingwu1225, while the details of this script are fresh on our minds, perhaps this is a good time to add unit and/or integration tests alongside the script? Integrating these tests within a GitHub Actions workflow would be straightforward and I can help with this process. Thanks for considering.
cc @balbasty
Hi @kabilar, thanks for the comment. I'll modify the script first based on comments of Yael and then we can start integrating tests, you help is appreciated! |
@kabilar @jingjingwu1225 We're about to merge the refactored package in main. It would be great to integrate the script and tests into this refactored package. @calvinchai can provide guidance. |
No description provided.