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

BF: Fix bug in example of reconst_csd.py #1477

Closed
wants to merge 16 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@RicciWoo
Contributor

RicciWoo commented Mar 25, 2018

This is to fix the error of example reconst_csd.py.

  1. transform image data from 1D to 4D before calling actor.odf_slicer();
  2. remove unexpected argument ‘scale' when calling actor.peak_slicer().

RicciWoo added some commits Mar 25, 2018

@codecov-io

This comment has been minimized.

codecov-io commented Mar 25, 2018

Codecov Report

Merging #1477 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1477      +/-   ##
==========================================
+ Coverage   87.51%   87.51%   +<.01%     
==========================================
  Files         241      241              
  Lines       30698    30698              
  Branches     3320     3320              
==========================================
+ Hits        26865    26866       +1     
  Misses       3058     3058              
+ Partials      775      774       -1
Impacted Files Coverage Δ
dipy/core/graph.py 75% <0%> (+1.19%) ⬆️

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 17140cd...7e656ca. Read the comment docs.

RicciWoo added some commits Mar 26, 2018

Merge pull request #4 from RicciWoo/fix-bug-sfm_reconst
delete unexpected argument scale
@@ -225,7 +227,7 @@
parallel=True)
window.clear(ren)
fodf_peaks = actor.peak_slicer(csd_peaks.peak_dirs, csd_peaks.peak_values, scale=1.3)
fodf_peaks = actor.peak_slicer(csd_peaks.peak_dirs, csd_peaks.peak_values)

This comment has been minimized.

@skoudoro

skoudoro Apr 4, 2018

Member

Why are you changing the scale here?

This comment has been minimized.

@RicciWoo

RicciWoo Apr 5, 2018

Contributor

Because actor.peak_slicer has no parameter called "scale".
I checked the file history, I realized that when "removing fvtk from this last examples" (on 10/6/2017),
we keep "scale=1.3", however, actor.peak_slicer() function doesn't have parameter called "scale":
before:
fodf_peaks = fvtk.peaks(csd_peaks.peak_dirs, csd_peaks.peak_values, scale=1.3)
changed to:
fodf_peaks = actor.peak_slicer(csd_peaks.peak_dirs, csd_peaks.peak_values, scale=1.3)
(now, line 230 in reconst_csd.py)
So I just remove this parameter, and it works. But if we need to the scale, we need to make change in actor.py.

This comment has been minimized.

@RicciWoo

RicciWoo Apr 7, 2018

Contributor

The same reason apply to PR #1479 in example sfm_reconst.py.

reconst_csa_parallel.py
reconst_csa.py
reconst_csd_parallel.py
# quick_start.py

This comment has been minimized.

@skoudoro

skoudoro Apr 4, 2018

Member

Can you remove all changes from this file?

This comment has been minimized.

@RicciWoo

RicciWoo Apr 5, 2018

Contributor

file removed.

RicciWoo added some commits Apr 5, 2018

Merge pull request #9 from nipy/master
incorporate upstream updates 20180409
Merge branch 'fix-bug-in-const_csd' of github.com:RicciWoo/dipy into …
…fix-bug-in-const_csd

* 'fix-bug-in-const_csd' of github.com:RicciWoo/dipy:
  change back to what it is on valid_examples.txt
  revert changes on valid_examples.txt
  Revert "update valid_examples.txt"

try to rebase again 20180409

@RicciWoo RicciWoo closed this Apr 11, 2018

@RicciWoo RicciWoo deleted the RicciWoo:fix-bug-in-const_csd branch Apr 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment