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

Plot both IVIM fits on the same axis #1806

Merged
merged 4 commits into from Apr 19, 2019

Conversation

@arokem
Copy link
Member

commented Apr 15, 2019

This is a small follow-up to #1789, in which the varpro and lm are plotted together.

@ShreyasFadnavis : could you please take a look?

@codecov-io

This comment has been minimized.

Copy link

commented Apr 15, 2019

Codecov Report

Merging #1806 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1806   +/-   ##
=======================================
  Coverage   83.95%   83.95%           
=======================================
  Files         120      120           
  Lines       14570    14570           
  Branches     2295     2295           
=======================================
  Hits        12232    12232           
  Misses       1820     1820           
  Partials      518      518
@ShreyasFadnavis

This comment has been minimized.

Copy link
Member

commented Apr 15, 2019

Thanks a lot @arokem !! LGTM 👍 @Garyfallidis @skoudoro can you merge?

@skoudoro

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

I still have the issue below (overlap) can you rebase your branch to have the last change @arokem? (it was fixed before)

measured_S0

@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

That's odd. That branch is up to date with master.

@skoudoro

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

Wow, ok, sorry for that. Can you update this plot too? Otherwise, this PR looks good to me too, I can merge it

@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

Update which plot? It looks like this when I run that example:

ivim_voxel_plot

@skoudoro

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

I agree this one is perfect, and this is the purpose of this PR. But I wonder whether you could fix the other one too (measure_S0.png on my previous comment) or if we should do it on a new PR.

@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

@pep8speaks

This comment has been minimized.

Copy link

commented Apr 19, 2019

Hello @arokem, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated at 2019-04-19 03:14:53 UTC
@arokem

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

OK. Should be fixed now!

@skoudoro

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Yes, I just tested it, thank you! merging

@skoudoro skoudoro merged commit ef24a38 into nipy:master Apr 19, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.