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

Upgrading to new pipeline #114

Merged
merged 80 commits into from Apr 17, 2014

Conversation

Projects
None yet
7 participants
@dmsurti
Contributor

dmsurti commented Jan 24, 2014

Step 1: Changing the pipeline topology. [IN PROGRESS]

Step 2: Changing the pipeline metadata

Step 3: Update only the algorithms, and not a combination of data objects/algorithms.

Step 4: (VTK 6 only): Subclass only from vtkAlgorithm and use the RequestXXXX api

@dmsurti

This comment has been minimized.

Contributor

dmsurti commented Feb 5, 2014

Upgrading to VTK 6. This will require VTK 6 on CI platform as well.
Closing for now.

@dmsurti dmsurti closed this Feb 5, 2014

@dmsurti

This comment has been minimized.

Contributor

dmsurti commented Mar 13, 2014

Reopening the PR, with these changes.

  1. tvtk module upgraded to vtk 6.
  2. tvtk module with backward compatibility changes for old pipeline.
  3. mayavi module with backward compatibility changes for old pipeline.

@dmsurti dmsurti reopened this Mar 13, 2014

Revert "Add logging info to handle mayavi.core logger not found"
This reverts commit d32b64b, which logging to
mayavi core.
@@ -211,11 +218,13 @@ def _texture_source_object_changed(self,old,new):
else:
self.actor.texture = None
self.texture.input = None
self.texture.input_connection = None

This comment has been minimized.

@prabhuramachandran

prabhuramachandran Mar 16, 2014

Member

Will this work with older pipelines? vtk-5.10.1 or 5.6 should work I guess.

This comment has been minimized.

@dmsurti

dmsurti Mar 18, 2014

Contributor

Yes, this will work with vtk 5.10.1 and 5.6.

if inp[0].has_output_port():
self.mapper.input_connection = inp[0].get_output_object()
else:
if is_old_pipeline():

This comment has been minimized.

@prabhuramachandran

prabhuramachandran Mar 16, 2014

Member

Why is this needed? The logic for this seems rather complex. Can't this block be wrapped into one reusable function which takes care of setting the input/input_connection from the appropriate output depending on the object being given? i.e. something like common.set_input(self.mapper, input) which encapsulates all of the logic. This makes the intent clear and puts all the logic in one place and also avoids repetition.

This comment has been minimized.

@dmsurti

dmsurti Mar 18, 2014

Contributor

Ok, I will look into relevant similar pieces of code and refactor them. Kept these as is, as would refactor these only after mayavi module is also updated for VTK 6, so that the refactor is once and for all.

@coveralls

This comment has been minimized.

coveralls commented Apr 17, 2014

Coverage Status

Changes Unknown when pulling ae4ec93 on dmsurti:new-pipeline into * on enthought:master*.

@coveralls

This comment has been minimized.

coveralls commented Apr 17, 2014

Coverage Status

Changes Unknown when pulling ae4ec93 on dmsurti:new-pipeline into * on enthought:master*.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build finished. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.enthought.com:8080/job/Mayavi_PR_builder/116/

Build Log
last 5 lines

[...truncated 15 lines...]
x86_64,Windows completed with result FAILURE
x86,Windows completed with result FAILURE
x86_64,MacOS completed with result UNSTABLE
x86,Linux completed with result UNSTABLE
@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build started. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build finished. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.enthought.com:8080/job/Mayavi_PR_builder/117/

Build Log
last 5 lines

[...truncated 13 lines...]
x86_64,Windows completed with result FAILURE
x86,Windows completed with result FAILURE
x86_64,MacOS completed with result UNSTABLE
x86,Linux completed with result UNSTABLE
@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build started. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build finished. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.enthought.com:8080/job/Mayavi_PR_builder/118/

Build Log
last 5 lines

[...truncated 13 lines...]
x86_64,Windows completed with result FAILURE
x86,Windows completed with result FAILURE
x86_64,MacOS completed with result UNSTABLE
x86,Linux completed with result UNSTABLE
@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build started. Test FAILed.

Fix many of the integration tests.
There are only two more that fail and these will be fixed in a
subsequent PR.
@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build finished. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.enthought.com:8080/job/Mayavi_PR_builder/119/

Build Log
last 5 lines

[...truncated 13 lines...]
x86_64,Windows completed with result FAILURE
x86,Windows completed with result FAILURE
x86_64,MacOS completed with result UNSTABLE
x86,Linux completed with result UNSTABLE
@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build triggered. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build started. Test FAILed.

@prabhuramachandran

This comment has been minimized.

Member

prabhuramachandran commented Apr 17, 2014

We now have Mayavi working with VTK-6.1. There were a few upstream bugs in VTK which have been fixed. The latest version of VTK master should work although VTK-6.1 + the following patches is well tested to work:

http://review.source.kitware.com/#/c/15095/1
http://review.source.kitware.com/#/c/15138/

With these, all the TVTK, mayavi tests pass and except 2 of the integration tests everything else passes. All the examples have also been tested and seem to run fine. This is the first phase of the migration to the new pipeline and once this is merged, we will work on the next set of changes to move entirely to the new pipeline.

Note, that VTK 5.10.1 continues to work fine as before.

@coveralls

This comment has been minimized.

coveralls commented Apr 17, 2014

Coverage Status

Changes Unknown when pulling 74bec8f on dmsurti:new-pipeline into * on enthought:master*.

@prabhuramachandran

This comment has been minimized.

Member

prabhuramachandran commented Apr 17, 2014

Travis is happy! Mayavi, now powered by VTK 6.x. ;-)

prabhuramachandran added a commit that referenced this pull request Apr 17, 2014

Merge pull request #114 from dmsurti/new-pipeline
Upgrading to new pipeline

@prabhuramachandran prabhuramachandran merged commit 72daa34 into enthought:master Apr 17, 2014

1 check passed

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

@prabhuramachandran prabhuramachandran deleted the dmsurti:new-pipeline branch Apr 17, 2014

@eric-jones

This comment has been minimized.

Member

eric-jones commented Apr 17, 2014

Congrats!

On Apr 17, 2014, at 11:57 AM, Prabhu Ramachandran notifications@github.com wrote:

Travis is happy! Mayavi, now powered by VTK 6.x. ;-)


Reply to this email directly or view it on GitHub.

@GaelVaroquaux

This comment has been minimized.

Collaborator

GaelVaroquaux commented Apr 17, 2014

Travis is happy! Mayavi, now powered by VTK 6.x. ;-)

Whooooot! Bravo

@opoplawski

This comment has been minimized.

opoplawski commented Apr 17, 2014

Excellent news. Any ETA on a new mayavi release then?

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Merged build finished. Test FAILed.

@enbuilder

This comment has been minimized.

enbuilder commented Apr 17, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.enthought.com:8080/job/Mayavi_PR_builder/120/

Build Log
last 5 lines

[...truncated 16 lines...]
x86,Windows completed with result FAILURE
x86_64,MacOS completed with result SUCCESS
x86,Linux appears to be cancelled
x86,Linux completed with result ABORTED
@prabhuramachandran

This comment has been minimized.

Member

prabhuramachandran commented Apr 17, 2014

No ETA on a release as there is still some work to do. This is the first major set of changes and while almost everything works there are some things that need to be fied.

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