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

MRG: update to plot_2d fixes and tests #543

Merged
merged 10 commits into from Jan 6, 2015

Conversation

Projects
None yet
4 participants
@matthew-brett
Member

matthew-brett commented Jan 6, 2015

Some more refactoring of plot_2d docstrings and another sampling_shape bugfix.
Refactor Ariel's smoke tests to work on travis.

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Jan 6, 2015

Omar - if the tests pass, and these changes look right to you, feel free to go ahead and merge.

@matthew-brett matthew-brett force-pushed the matthew-brett:align-fixes branch from 76236bb to fc1bfd5 Jan 6, 2015

matthew-brett added some commits Jan 6, 2015

RF: add matplotlib rc file to disable figures
To allow matplotlib figure tests on travis.
DOC+RF: rewrite, expand docstrings, reformat
Docstring rewrites, PEP8 formatting.
BF+TST: fix sampling_shape again; fix smoke tests
Fix another instance of sampling shape as a tuple; fix and extend
Ariel's smoke tests.

@matthew-brett matthew-brett force-pushed the matthew-brett:align-fixes branch from fc1bfd5 to 827fef3 Jan 6, 2015

matthew-brett added some commits Jan 6, 2015

RF: adapt dipy test runner to possibility of mpl
Make dipy test runner import matplotlib with 'agg' backend, so that mpl
tests don't open figures by default.

This will help the buildbots run the matplotlib tests.
RF: integer division in making lattice grid
Use integer division explicitly for making lattice grid (fixes Python 3
test failure).
lattice_out=lattice_out[0:inverse_grid_shape[0], 0:inverse_grid_shape[1]]
# Draw the squares on the output grid
lattice_out = draw_lattice_2d(
(inverse_grid_shape[0] + delta) // (delta + 1),

This comment has been minimized.

@matthew-brett

matthew-brett Jan 6, 2015

Member

Omar - is this correct? Integer division (rounding down)?

This comment has been minimized.

@omarocegueda

omarocegueda Jan 6, 2015

Contributor

sorry matthew, i dont have my computer right now, i will come back within one hour an take a look!

-----Original Message-----
From: "Matthew Brett" notifications@github.com
Sent: ‎1/‎6/‎2015 9:39
To: "nipy/dipy" dipy@noreply.github.com
Subject: Re: [dipy] MRG: update to plot_2d fixes and tests (#543)

In dipy/viz/regtools.py:

  • #Draw the squares on the output grid
  • X1,X0 = np.mgrid[0:inverse_grid_shape[0], 0:inverse_grid_shape[1]]
  • lattice_out=draw_lattice_2d((inverse_grid_shape[0] + delta) / (delta + 1),
  •                            (inverse_grid_shape[1] + delta) / (delta + 1),
    
  •                            delta)
    
  • lattice_out=lattice_out[0:inverse_grid_shape[0], 0:inverse_grid_shape[1]]
  • Draw the squares on the output grid

  • lattice_out = draw_lattice_2d(
  •    (inverse_grid_shape[0] + delta) // (delta + 1),
    
    Omar - is this correct? Integer division (rounding down)?

    Reply to this email directly or view it on GitHub.=

This comment has been minimized.

@omarocegueda

omarocegueda Jan 6, 2015

Contributor

Hi @matthew-brett, =)
thank you very much for reviewing this!, this is correct, it is actually integer division rounding up: (x+(n-1))//n is division by n rounding up. This considers the case delta=0 (the degenerate case where there is no separation between division lines).

delta)
lattice_in=lattice_in[0:direct_grid_shape[0], 0:direct_grid_shape[1]]
# Draw the squares on the input grid
lattice_in = draw_lattice_2d((direct_grid_shape[0] + delta) // (delta + 1),

This comment has been minimized.

@matthew-brett

matthew-brett Jan 6, 2015

Member

Ditto - is integer division the right thing to do here?

This comment has been minimized.

@omarocegueda

omarocegueda Jan 6, 2015

Contributor

Yes, it is safer not to deal with floating point operations when possible (we might get different results depending on the precision of the floats being used, like the double vs. extended discussion we had some time ago)

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Jan 6, 2015

Tests passing, this one is ready to go from my point of view.

@omarocegueda

This comment has been minimized.

omarocegueda commented on 2884596 Jan 6, 2015

Thank you Ariel! =)

omarocegueda added a commit that referenced this pull request Jan 6, 2015

@omarocegueda omarocegueda merged commit c3e1d37 into nipy:master Jan 6, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@arokem arokem referenced this pull request Jan 6, 2015

Closed

TST: Testing regtools #542

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