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

[fix] fix value_range in HORIZON #2249

Merged
merged 2 commits into from
Oct 7, 2020
Merged

Conversation

skoudoro
Copy link
Member

@skoudoro skoudoro commented Oct 5, 2020

This PR fix #2243.

Sometimes, the range value of the first dataset volume is empty/null. This is the case with our isbi2013_3shell dataset.
So the goal of this fix is to select the volume with a relevant value range for the HORIZON visualization

@skoudoro skoudoro added this to the 1.3 milestone Oct 5, 2020
@skoudoro
Copy link
Member Author

skoudoro commented Oct 5, 2020

Can you have a quick check @areeshatariq and @jhlegarreta?

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #2249 into master will decrease coverage by 0.01%.
The diff coverage is 10.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2249      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.02%     
==========================================
  Files         252      252              
  Lines       32933    32941       +8     
  Branches     3462     3465       +3     
==========================================
+ Hits        30095    30097       +2     
- Misses       2079     2085       +6     
  Partials      759      759              
Impacted Files Coverage Δ
dipy/viz/panel.py 64.16% <10.00%> (-1.46%) ⬇️
dipy/segment/tests/test_bundles.py 89.52% <0.00%> (-0.96%) ⬇️
dipy/viz/app.py 77.47% <0.00%> (+0.49%) ⬆️

Copy link
Contributor

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

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

Have not tested locally, but looks a fair solution. Thanks for doing this.

@jhlegarreta
Copy link
Contributor

jhlegarreta commented Oct 7, 2020

CI build failures (related to SSL certificates during the package installation steps) are unrelated to this patch set. We trust the passing builds. Merging.

@jhlegarreta jhlegarreta merged commit a12a46a into dipy:master Oct 7, 2020
@skoudoro skoudoro deleted the issue-2243 branch October 8, 2020 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to visualize data through dipy_horizon
3 participants