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

Issue349 imtool3 d td #355

Merged
merged 10 commits into from Sep 6, 2019
Merged

Issue349 imtool3 d td #355

merged 10 commits into from Sep 6, 2019

Conversation

tanguyduval
Copy link
Member

@tanguyduval tanguyduval commented Sep 5, 2019

Purpose

solve #349

Approach

in nii_load.m, Apply use the scaling and convert to double automatically. If output takes more than 300Mb of memory consumption ask the user.

These changes were already done in dicm2nii and imtool3D. So I just updated these two External repo with their latest version.

Open Questions and Pre-Merge TODOs

  • Use github checklists. When solved, check the box and explain the answer.

  • Review that changed source files/lines are related to the pull request/issue
    If any files/commits were accidentally included, cherry-pick them into another branch.

  • Review that changed source files/lines were not accidentally deleted
    Fix appropriately if so.

  • Test new features or bug fix
    If not implemented/resolved adequately, solve it or inform the developer by requesting changes in your review.
    Preferably, set breakpoints in the locations that the code was changed and follow allong line by line to see if the code behaves as intended.

Manual GUI tests (general)
  • Does the qMRLab GUI open?
  • Can you change models?
  • Can you load a data folder for a model?
  • Can you view data?
  • Can you zoom in the image?
  • Can you pan out of the image?
  • Can you view the histogram of the data?
  • Can you change the color map?
  • Can you fit dataset (Fit data)?
  • Can you save/load the results?
  • Can you open the options panel?
  • Can you change option parameters?
  • Can you save/load option paramters?
  • Can you select a voxel?
  • Can you fit the data of that voxel ("View data fit")?
  • Can you simulate and fit a voxel ("Single Voxel Curve")?
  • Can you run a Sensitivity Analysis?
  • Can you simulate a Multi Voxel Distribution?
Specifications
  • Version: MATLAB 2018b
  • Platform: MacBook 10.14.6
Reviewer

@mathieuboudreau

@mathieuboudreau
Copy link
Member

Great! Thanks @tanguyduval ! What's your thoughts on handling the conflicting files? It appears the y are too complex to view on GitHub.

Should we just use the ones in this branch to overwrite the one in master? Or are the ones in master possibly modified from the base imtool3D ones, and overwriting them would result in missing features/lines of code neeed for your _td mods?

# Conflicts:
#	External/imtool3D_td/imtool3D.m
#	External/imtool3D_td/imtool3D_3planes.m
#	External/imtool3D_td/imtool3D_nii_3planes.m
#	External/imtool3D_td/src/load_nii_datas.m
@tanguyduval
Copy link
Member Author

@mathieuboudreau I just did the merge. I used the new version and overwrote master with new versions of imtool3D.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.006%) to 43.036% when pulling cf6289d on issue349_imtool3D_td into e05e0fc on master.

Copy link
Member

@mathieuboudreau mathieuboudreau left a comment

Choose a reason for hiding this comment

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

This PR perfectly fixes #349. I went through the PR review checklist and everything pass fine.

Thanks a lot @tanguyduval !!

@mathieuboudreau mathieuboudreau merged commit 545e26f into master Sep 6, 2019
@mathieuboudreau mathieuboudreau deleted the issue349_imtool3D_td branch September 6, 2019 18:42
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

3 participants