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

[imaging_browser] Modify content of table displaying image headers information #8157

Merged

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Aug 24, 2022

Brief summary of changes

This PR modifies the information displayed in the headers table. Modifications include:

  • addition of Image Type, Phase Encoding Direction, Echo Number and Inversion Time parameters to the table
  • addition of Processing Pipeline and Processing Date in the list of displayed header if those contain values
  • removal of pipeline and algorithm headers since they have been removed from the DB a few years ago and therefore will always be empty
  • display DTIPrep specific headers only for images that has ProcessingPipeline set to 'DTIPrepPipeline'
  • if Number of Volumes is 0, do not display any value instead of showing 0.00 volumes
  • for Number of Volumes that are not set to 0, display the number of volumes as integer instead of floats (example: 200 volumes instead of 200.00 volumes)

Testing instructions

  1. Check out this branch, run make dev and look at the header table.
  2. Volumes should be displayed only for fMRI or DWI data that have multiple volumes and should be displayed as integer instead of floats
  3. Ensure that the new header information are showing when the scan have parameters for those headers
  4. Ensure that the default DTIPrep headers are not showing for images that are not produced by DTIPrepPipeline

Link(s) to related issue(s)

@cmadjar cmadjar added the Category: Feature PR or issue that aims to introduce a new feature label Aug 24, 2022
@driusan
Copy link
Collaborator

driusan commented Aug 26, 2022

Is there a reason this needs to go to 24.1 instead of main? We should be prioritizing releasing 24.1 at this point..

@cmadjar
Copy link
Collaborator Author

cmadjar commented Sep 20, 2022

@driusan this is to avoid any overrides in HBCD. Also, the PR fixes a lot of different bugs in the table displaying the headers...

@nicolasbrossard
Copy link
Contributor

@cmadjar I might be having an issue with the processing date but wanted to confirm with you. I added a dummy record in parameter_file for one of the files on raisin bread and set the value to 20140206 (that's how the processing dates are stored in IBIS). When I looked at the headers, the pipeline processing date displayed was 1391706000 (most likely the Unix timestamp representation of that date). Bug?

@cmadjar cmadjar force-pushed the modify_imaging_browser_headers_displayed branch from 2ea6f72 to b793d30 Compare September 22, 2022 13:26
@cmadjar
Copy link
Collaborator Author

cmadjar commented Sep 22, 2022

@nicolasbrossard weird. It was getting the date via the _getDate function of viewsession.class.inc which converts dates to timestamps for some reason. Not sure about the utility of that function... Anyway, I corrected the processing date so that it displays what is present in the parameter_file table.

I also resolved the conflict with the CHANGELOG.

@nicolasbrossard
Copy link
Contributor

@cmadjar Works fine now. Approving.

@nicolasbrossard nicolasbrossard added the Passed manual tests PR has been successfully tested by at least one peer label Sep 22, 2022
@driusan driusan merged commit 8c27401 into aces:24.1-release Sep 22, 2022
@ridz1208 ridz1208 added this to the 24.1.0 milestone Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Feature PR or issue that aims to introduce a new feature Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants