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

Fvtk 2.0 PR1 #587

Merged
merged 258 commits into from Nov 1, 2015
Merged

Fvtk 2.0 PR1 #587

merged 258 commits into from Nov 1, 2015

Conversation

Garyfallidis
Copy link
Contributor

This will make you happy @arokem and @stefanv! Moving forward with the visualization capabilities of DIPY!!! The PR is ready for a review.

This pull request introduces a new API for visualization.

Here are the main characteristics:
a) Allows for testable screenshots and renderers. Which is crucial for quality assurance.
b) It contains 3 main modules (window, actor and widget). These are replacing the current fvtk module but fvtk can be used too for backwards compatibility.
c) In window we have everything related to windowing, rendering and saving screenshots.
d) In actor you will find the actors for lines and streamtubes etc. Not everything has moved yet from fvtk.py so that the PR doesn't become too large. But all actors should move in the next PRs.
e) In widget we have buttons and sliders which can be used directly from inside OpenGL/VTK. No new dependencies.
d) You can save and open file dialogues using Tk which is ported together with Python. No new dependencies.
f) Because they are a lot of new concepts and ideas in this PR I wrote 4 tutorials to help the reviewing process.

Let me know what you think.

@arokem
Copy link
Contributor

arokem commented Oct 30, 2015

Oh no - wait - you're right. There's one: "No VTK for these tests" in there
as well.

Let me double check again...

On Thu, Oct 29, 2015 at 7:24 PM, Ariel Rokem arokem@gmail.com wrote:

Yeah - those are other test-conditions that are causing those to be
skipped. I think this is about importing the vtk colors module. Maybe we
can take that separately?

On Thu, Oct 29, 2015 at 6:15 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

@arokem https://github.com/arokem just to make sure here. On top of
which branch do I need to rebase this one? Is it #752
#752? Because I see some vtk tests
are skipped there. But maybe this is normal for that version of the code.
Sorry in advance for the naivety of the question, it's getting late here
and I had a long day. :) But hey no excuses! Let's get the new viz in!


Reply to this email directly or view it on GitHub
#587 (comment).

@arokem
Copy link
Contributor

arokem commented Oct 30, 2015

OK - I am about ready to say that we can't test some of these things on
Travis. I propose setting this to skip on Travis.

If you rebase this on master, I can make a PR against your branch with the
proposed changes to make that happen.

On Thu, Oct 29, 2015 at 7:45 PM, Ariel Rokem arokem@gmail.com wrote:

Oh no - wait - you're right. There's one: "No VTK for these tests" in
there as well.

Let me double check again...

On Thu, Oct 29, 2015 at 7:24 PM, Ariel Rokem arokem@gmail.com wrote:

Yeah - those are other test-conditions that are causing those to be
skipped. I think this is about importing the vtk colors module. Maybe we
can take that separately?

On Thu, Oct 29, 2015 at 6:15 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

@arokem https://github.com/arokem just to make sure here. On top of
which branch do I need to rebase this one? Is it #752
#752? Because I see some vtk tests
are skipped there. But maybe this is normal for that version of the code.
Sorry in advance for the naivety of the question, it's getting late here
and I had a long day. :) But hey no excuses! Let's get the new viz in!


Reply to this email directly or view it on GitHub
#587 (comment).

Garyfallidis and others added 8 commits October 30, 2015 13:02
arokem added a commit that referenced this pull request Nov 1, 2015
@arokem arokem merged commit d90ee2a into dipy:master Nov 1, 2015
@arokem
Copy link
Contributor

arokem commented Nov 1, 2015

Congratulations @Garyfallidis! A new era of diffusion MRI visualization!

@Garyfallidis
Copy link
Contributor Author

Oh man it has been a looooong trip! I am so pleased :) Things are getting exciting! @MarcCote first dipy.viz PR is in!!! You know exactly what that means! Mayhem!!! @arokem many thx for the great help on setting up VTK on mad Travis. 👯

@Garyfallidis
Copy link
Contributor Author

Let's start making some smaller viz PRs from now on. 👍

@arokem
Copy link
Contributor

arokem commented Nov 1, 2015

+1

On Sun, Nov 1, 2015 at 2:04 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

Let's start making some smaller viz PRs from now on. [image: 👍] This
one was a bit too big.


Reply to this email directly or view it on GitHub
#587 (comment).

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

8 participants