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

bugfix: erroneous implementation of reading RGB nifti #23

Closed
wants to merge 3 commits into from

Conversation

xgui3783
Copy link
Collaborator

@xgui3783 xgui3783 commented Feb 7, 2022

bugfix: rgb nifti bug
tests: added tests for arbitary dimension rgb nii
tests: added testing values before and after volume_file_to_precomputed

previous implementation of reading rgb nii was errorneous.

tests: added tests for arbitary dimension rgb nii
tests: added testing values before and after volume_file_to_precomputed

previous implementation of reading rgb nii was errorneous.
@xgui3783 xgui3783 requested a review from ylep February 7, 2022 15:08
@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #23 (43e5faa) into master (b63b2bd) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   92.40%   92.43%   +0.02%     
==========================================
  Files          25       25              
  Lines        1475     1480       +5     
  Branches      211      230      +19     
==========================================
+ Hits         1363     1368       +5     
  Misses         63       63              
  Partials       49       49              
Impacted Files Coverage Δ
...r_scripts/scripts/volume_to_precomputed_pyramid.py 89.58% <100.00%> (-0.22%) ⬇️
src/neuroglancer_scripts/volume_reader.py 83.33% <100.00%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b63b2bd...43e5faa. Read the comment docs.

@xgui3783 xgui3783 removed the request for review from ylep February 7, 2022 15:12
@xgui3783 xgui3783 marked this pull request as draft February 7, 2022 15:12
@xgui3783 xgui3783 marked this pull request as ready for review February 7, 2022 16:25
@xgui3783 xgui3783 requested a review from ylep February 7, 2022 16:26
@xgui3783
Copy link
Collaborator Author

xgui3783 commented Feb 7, 2022

Apologies for the spam. I noticed that there (still) were some error in the code (turns out, the byte order matters very much how ndarray.view collapses rgb nifti.

The PR should be ready for review now.

@xgui3783 xgui3783 marked this pull request as draft February 7, 2022 16:58
@xgui3783 xgui3783 removed the request for review from ylep February 8, 2022 09:11
@xgui3783
Copy link
Collaborator Author

xgui3783 commented Feb 8, 2022

Apologies. There are still some issues that I am trying to sort out re this PR. I will mark ready for review & tag you when it's ready.

test: test nibabel.load proxy
@ylep
Copy link
Collaborator

ylep commented Mar 7, 2023

Hi @xgui3783, what is the status of this PR? Is it still relevant? If so, do you see any chance to get it ready before we release version 1.1.0?

@xgui3783
Copy link
Collaborator Author

xgui3783 commented Mar 7, 2023

@ylep , sorry, still relevant, but no progress.

It was brought to me attention that, since there are no unanimous format on how rgb is encoded in nifti, different softwares seems to use their own system to encode the rgb channel.

Perhaps I could close thie PR and add an issue instead.

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.

None yet

2 participants