Fix "_p_void" values in TVTK with VTK 6.x #370

Merged
merged 25 commits into from May 13, 2016

Conversation

Projects
None yet
4 participants
@kitchoi
Member

kitchoi commented Apr 29, 2016

I have tested this with VTK 6.2 and 6.3 on Ubuntu 12, and VTK 6.3 on OSX 10.11

Things seem to work but since this is a fundamental change, I would prefer sitting on it for a bit longer. Review and comments are very much appreciated.

Most of these changes are tested by the existing TVTK test for initialising all classes, but I think I could also add more tests to make it more robust.

To compare the bahaviour, try the following with the master branch and with this one:

from tvtk.api import tvtk
obj = tvtk.ViewTheme()
obj.configure_traits()
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Apr 29, 2016

Current coverage is 45.53%

Merging #370 into master will decrease coverage by -<.01%

@@             master       #370   diff @@
==========================================
  Files           258        258          
  Lines         22909      23011   +102   
  Methods           0          0          
  Messages          0          0          
  Branches       3030       3036     +6   
==========================================
+ Hits          10451      10476    +25   
- Misses        11729      11805    +76   
- Partials        729        730     +1   
  1. File ...ayavi/core/engine.py (not in diff) was modified. more

Powered by Codecov. Last updated by 10f5db9...c1e1f46

codecov-io commented Apr 29, 2016

Current coverage is 45.53%

Merging #370 into master will decrease coverage by -<.01%

@@             master       #370   diff @@
==========================================
  Files           258        258          
  Lines         22909      23011   +102   
  Methods           0          0          
  Messages          0          0          
  Branches       3030       3036     +6   
==========================================
+ Hits          10451      10476    +25   
- Misses        11729      11805    +76   
- Partials        729        730     +1   
  1. File ...ayavi/core/engine.py (not in diff) was modified. more

Powered by Codecov. Last updated by 10f5db9...c1e1f46

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi Apr 29, 2016

Member

Should fix #357 and #360

Member

kitchoi commented Apr 29, 2016

Should fix #357 and #360

tvtk/tvtk_base.py
@@ -272,6 +274,11 @@ class TVTKBase(traits.HasStrictTraits):
# The wrapped VTK object.
_vtk_obj = traits.Trait(None, None, vtk.vtkObjectBase())
+ # Stores the names of the traits whose VTK Get methods may return
+ # invalid values (e.g. reference to a point) or uninitialised values
+ # We would try to update but allow it to failure

This comment has been minimized.

@prabhuramachandran

prabhuramachandran Apr 29, 2016

Member

"allow it to fail" :)

@prabhuramachandran

prabhuramachandran Apr 29, 2016

Member

"allow it to fail" :)

tvtk/wrapper_gen.py
+
+
+def get_array_meta(value):
+ if not isinstance(value, Sequence):

This comment has been minimized.

@kitchoi

kitchoi Apr 29, 2016

Member

self-review: need docstring here

@kitchoi

kitchoi Apr 29, 2016

Member

self-review: need docstring here

@kitchoi kitchoi changed the title from Fix "_p_void" values in TVTK with VTK 6.x [WIP] to Fix "_p_void" values in TVTK with VTK 6.x May 4, 2016

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 4, 2016

Contributor

This is too much code to review without being told more.

Contributor

stefanoborini commented May 4, 2016

This is too much code to review without being told more.

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 4, 2016

Member

If you try to run the new tvtk test case with the master branch + VTK 6.3, you should see a bunch of failures: https://gist.github.com/kitchoi/8e92ad08e44334133360536ce3d5bdff
These are what this PR is fixing, plus a bunch of refactoring.

Member

kitchoi commented May 4, 2016

If you try to run the new tvtk test case with the master branch + VTK 6.3, you should see a bunch of failures: https://gist.github.com/kitchoi/8e92ad08e44334133360536ce3d5bdff
These are what this PR is fixing, plus a bunch of refactoring.

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 4, 2016

Contributor

The problem now is that it's very hard to differentiate between what you changed for the fix and what you refactored.

Contributor

stefanoborini commented May 4, 2016

The problem now is that it's very hard to differentiate between what you changed for the fix and what you refactored.

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 4, 2016

Contributor

I think it's time we ditch codecov until they fix this unexpected changes problem.

Contributor

stefanoborini commented May 4, 2016

I think it's time we ditch codecov until they fix this unexpected changes problem.

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 4, 2016

Member

If you want less of the refactoring changes, try git diff the master and this commit: 7a30bdd

Member

kitchoi commented May 4, 2016

If you want less of the refactoring changes, try git diff the master and this commit: 7a30bdd

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 4, 2016

Contributor

Try pushing a dummy change. I disabled codecov

Contributor

stefanoborini commented May 4, 2016

Try pushing a dummy change. I disabled codecov

- if klass.__name__ == 'vtkCubeAxesActor2D' and \
- name == 'inertia':
- # VTK bug. Inconsistent API!
- rng = (float(rng[0]), float(rng[1]))

This comment has been minimized.

@kitchoi

kitchoi May 4, 2016

Member

I have removed the special handling of 'vtkCubeAxesActor2D.inertia' here because from what I have researched on the VTK API, inertia is indeed an int and there is no inconsistency.

@kitchoi

kitchoi May 4, 2016

Member

I have removed the special handling of 'vtkCubeAxesActor2D.inertia' here because from what I have researched on the VTK API, inertia is indeed an int and there is no inconsistency.

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

It is quite possible this was a bug with VTK 4.x or so which is not relevant anymore so this is fine.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

It is quite possible this was a bug with VTK 4.x or so which is not relevant anymore so this is fine.

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 4, 2016

Contributor

apparently the codecov yaml file did not solve for this branch

Contributor

stefanoborini commented May 4, 2016

apparently the codecov yaml file did not solve for this branch

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 5, 2016

Member

I think the "codecov/project" captures the increase in coverage after we added VTK 6.3 and VTK 7 in the test matrix, it raises a red flag because it did not expect those changes (who knows how they decide what they should expect).

Member

kitchoi commented May 5, 2016

I think the "codecov/project" captures the increase in coverage after we added VTK 6.3 and VTK 7 in the test matrix, it raises a red flag because it did not expect those changes (who knows how they decide what they should expect).

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 5, 2016

Member

That "unexpected changes by codecov", I guess, has to do with the fact that this PR started from a master before VTK 6.3 and VTK 7 were added to travis. So I guess this behaviour is only temporary, i.e. future PRs should not have this problem.
Parallel to this, if we all agree that the "codecov/changes" check is not required, we can raise a new issue and fix can be done by adding status: changes: enabled: no in the codcov.yml (see https://github.com/codecov/support/wiki/codecov.yml)

Member

kitchoi commented May 5, 2016

That "unexpected changes by codecov", I guess, has to do with the fact that this PR started from a master before VTK 6.3 and VTK 7 were added to travis. So I guess this behaviour is only temporary, i.e. future PRs should not have this problem.
Parallel to this, if we all agree that the "codecov/changes" check is not required, we can raise a new issue and fix can be done by adding status: changes: enabled: no in the codcov.yml (see https://github.com/codecov/support/wiki/codecov.yml)

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 5, 2016

Contributor

Exactly, I open a PR to remove the change. I'll test it on my PR for OSX and then you can merge it here if it works.

Contributor

stefanoborini commented May 5, 2016

Exactly, I open a PR to remove the change. I'll test it on my PR for OSX and then you can merge it here if it works.

+ errors_trait_is_ptr.append(
+ (tvtk_klass_name, trait_name, trait))
+
+ if errors_trait_is_ptr:

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

if errors_getting_trait

@stefanoborini

stefanoborini May 5, 2016

Contributor

if errors_getting_trait

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

errors_trait_is_ptr should be correct now.

@kitchoi

kitchoi May 5, 2016

Member

errors_trait_is_ptr should be correct now.

tvtk/tests/test_tvtk.py
+ '\n'.join(('{0}.{1} = {2}'.format(*error)
+ for error in errors_trait_is_ptr)))
+
+ def test_no_trait_has_ptr_address_as_value(self):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Seems this method and the above have the same name.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Seems this method and the above have the same name.

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

Oops, my copypasta, well spotted.

@kitchoi

kitchoi May 5, 2016

Member

Oops, my copypasta, well spotted.

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

The first test_no_trait_has_ptr_address_as_value is a ghost copypasta that was not deleted, deleting it now.

@kitchoi

kitchoi May 5, 2016

Member

The first test_no_trait_has_ptr_address_as_value is a ghost copypasta that was not deleted, deleting it now.

+ for error in errors_trait_is_ptr))
+ self.fail(message)
+
+ def test_all_traits_can_be_obtained(self):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Is this one redundant with two methods above?

@stefanoborini

stefanoborini May 5, 2016

Contributor

Is this one redundant with two methods above?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

removed the first one which is the actual duplicate.

@kitchoi

kitchoi May 5, 2016

Member

removed the first one which is the actual duplicate.

@@ -471,15 +478,28 @@ def update_traits(self, obj=None, event=None):
vtk.vtkObject.GlobalWarningDisplayOff()
for name, getter in self._updateable_traits_:
+ if name == 'global_warning_display':

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

what about this one ? what makes it special?

@stefanoborini

stefanoborini May 5, 2016

Contributor

what about this one ? what makes it special?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

This one is not originally my code, I just moved it out of the try block. So I don't know.

@kitchoi

kitchoi May 5, 2016

Member

This one is not originally my code, I just moved it out of the try block. So I don't know.

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Ok. Leave it as is. We can't know everything.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Ok. Leave it as is. We can't know everything.

@@ -295,7 +294,8 @@ def get_other_methods(self):
"""
return self.other_meths
- def get_method_signature(self, method):
+ @staticmethod
+ def get_method_signature(method):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

function?

@stefanoborini

stefanoborini May 5, 2016

Contributor

function?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

There are many occurrence of parser = VTKMethodParser(); parser.get_method_signature(...) in wrapper_gen and in tests (and possibly some other places I have not touched), could do with a separately cleanup PR in order to move this function out of the class?

@kitchoi

kitchoi May 5, 2016

Member

There are many occurrence of parser = VTKMethodParser(); parser.get_method_signature(...) in wrapper_gen and in tests (and possibly some other places I have not touched), could do with a separately cleanup PR in order to move this function out of the class?

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Leave it as is. not top priority. If you want, open a issue tagged refactoring.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Leave it as is. not top priority. If you want, open a issue tagged refactoring.

tvtk/wrapper_gen.py
from . import vtk_parser
from . import indenter
from . import special_gen
PY_VER = sys.version_info[0]
+def get_trait_def(value, kwargs="enter_set=True, auto_set=False"):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

this is a bit weird... when i see kwargs I immediately assume dict.

@stefanoborini

stefanoborini May 5, 2016

Contributor

this is a bit weird... when i see kwargs I immediately assume dict.

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 12, 2016

Member

Yes, I agree this looks a bit unusual.

@prabhuramachandran

prabhuramachandran May 12, 2016

Member

Yes, I agree this looks a bit unusual.

This comment has been minimized.

@kitchoi

kitchoi May 12, 2016

Member

I overlooked this comment. Will make kwargs default to a dict, and compose the string within the function.

@kitchoi

kitchoi May 12, 2016

Member

I overlooked this comment. Will make kwargs default to a dict, and compose the string within the function.

+
+ Parameters
+ ----------
+ value

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

how about

value
    Can be anything
@kitchoi

kitchoi May 5, 2016

Member

how about

value
    Can be anything

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

You pretty much have to put something, even trivial, I guess? if we run the docgen on it I think it will complain (do we even run the docgen on it?). Good for me.

@stefanoborini

stefanoborini May 5, 2016

Contributor

You pretty much have to put something, even trivial, I guess? if we run the docgen on it I think it will complain (do we even run the docgen on it?). Good for me.

+ default = default_mappings[arg_format[0]]
+
+ if len(arg_format) > 1:
+ return (default,)*len(arg_format)

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

How does it react to something having (int, float)?

@stefanoborini

stefanoborini May 5, 2016

Contributor

How does it react to something having (int, float)?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

all_same_type is False

@kitchoi

kitchoi May 5, 2016

Member

all_same_type is False

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

I meant in general.... are those cases handled at all (doesn't seem so)?

@stefanoborini

stefanoborini May 5, 2016

Contributor

I meant in general.... are those cases handled at all (doesn't seem so)?

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

For those cases, the vtk set method and get method are written as is. This is exactly what test_spider_plot_actor in tvtk/test_tvtk.py is testing.

@kitchoi

kitchoi May 5, 2016

Member

For those cases, the vtk set method and get method are written as is. This is exactly what test_spider_plot_actor in tvtk/test_tvtk.py is testing.

+ }
+
+ @classmethod
+ def _get_special_matched_key(cls, klass, vtk_attr_name):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

I generally don't mind docstrings for methods that are self-descriptive, such as the one above, but this one probably needs explanation.

@stefanoborini

stefanoborini May 5, 2016

Contributor

I generally don't mind docstrings for methods that are self-descriptive, such as the one above, but this one probably needs explanation.

tvtk/wrapper_gen.py
+ return cls.special_traits.get(key)[:2]
+
+ @classmethod
+ def _is_special(cls, klass, vtk_attr_name):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

same

@stefanoborini

stefanoborini May 5, 2016

Contributor

same

tvtk/wrapper_gen.py
+ self._write_trait(out, name, t_def, vtk_set_meth, mapped=False)
+
+ def _write_hardware_selector_prop_color_value(self, klass, out,
+ vtk_attr_name='PropColorValue'):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Weird that you have to pass a default value.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Weird that you have to pass a default value.

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

I don't have to. But I think it acts as a form of documentation.

@kitchoi

kitchoi May 5, 2016

Member

I don't have to. But I think it acts as a form of documentation.

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

I understand the intent, but it's deceiving. When I see that, I don't expect it to be documentation.
You already enforce the value in the content. When I see that I assume that the method can be called without any passed vtk_attr_name for some reason, and you are "fixing" that behavior with the only acceptable value, but the vtk_attr_name is always passed, only in this routine must be one and only one value, otherwise there's a programming error.

@stefanoborini

stefanoborini May 5, 2016

Contributor

I understand the intent, but it's deceiving. When I see that, I don't expect it to be documentation.
You already enforce the value in the content. When I see that I assume that the method can be called without any passed vtk_attr_name for some reason, and you are "fixing" that behavior with the only acceptable value, but the vtk_attr_name is always passed, only in this routine must be one and only one value, otherwise there's a programming error.

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

I have removed the default value now. Unfortunately I have to keep the signature consistent, so vtk_attr_name is kept there.

@kitchoi

kitchoi May 5, 2016

Member

I have removed the default value now. Unfortunately I have to keep the signature consistent, so vtk_attr_name is kept there.

tvtk/wrapper_gen.py
+ force_update='False')
+
+ def _write_axes_transform_representation_tolerance(self, klass, out,
+ vtk_attr_name='Tolerance'):

This comment has been minimized.

@stefanoborini

stefanoborini May 5, 2016

Contributor

Same

@stefanoborini

stefanoborini May 5, 2016

Contributor

Same

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 5, 2016

Contributor

Review ready. Merge when done (they are mostly minor).

Contributor

stefanoborini commented May 5, 2016

Review ready. Merge when done (they are mostly minor).

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 5, 2016

Contributor

How is this related to issue #385 ? did you skip the test?

Contributor

stefanoborini commented May 5, 2016

How is this related to issue #385 ? did you skip the test?

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 5, 2016

Member

What is issue #907?

Member

kitchoi commented May 5, 2016

What is issue #907?

@stefanoborini

This comment has been minimized.

Show comment
Hide comment
@stefanoborini

stefanoborini May 5, 2016

Contributor

Fixed. I pasted the wrong content.

Contributor

stefanoborini commented May 5, 2016

Fixed. I pasted the wrong content.

+ # e.g. x = numpy.empty(9); ImageConvolve.GetKernel3x3(x)
+ # FIXME: we could try harder in retrieving these values
+ # in TVTKBase.update_traits
+ del updateable_traits[name]

This comment has been minimized.

@kitchoi

kitchoi May 5, 2016

Member

If I change del updateable_traits[name] to allow_update_failure.add(name), the difference in behaviour is not only that TVTKBase.update_traits will have more updating to do while those updating will almost certainly fail with TraitError, but I will also get these traits visible on the UI.
E.g. with del updateable_traits[name]
obj = tvtk.ImageConvolve(); obj.configure_traits()
del_updateable

E.g. replace with allow_update_failure.add(name)
allow

Question is, which one do we want. The bahaviour on the interactive interpreter is the same.

@kitchoi

kitchoi May 5, 2016

Member

If I change del updateable_traits[name] to allow_update_failure.add(name), the difference in behaviour is not only that TVTKBase.update_traits will have more updating to do while those updating will almost certainly fail with TraitError, but I will also get these traits visible on the UI.
E.g. with del updateable_traits[name]
obj = tvtk.ImageConvolve(); obj.configure_traits()
del_updateable

E.g. replace with allow_update_failure.add(name)
allow

Question is, which one do we want. The bahaviour on the interactive interpreter is the same.

This comment has been minimized.

@kitchoi

kitchoi May 6, 2016

Member

Corran suggested using TextEditor for these arrays. Unfortunately I am preoccupied at the moment. I think I would attack this problem in a separate PR if and after this one is merged.

@kitchoi

kitchoi May 6, 2016

Member

Corran suggested using TextEditor for these arrays. Unfortunately I am preoccupied at the moment. I think I would attack this problem in a separate PR if and after this one is merged.

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Fair enough. Maybe adding an issue for that would be good so we don't forget.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Fair enough. Maybe adding an issue for that would be good so we don't forget.

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 5, 2016

Member

How is this related to issue #385 ? did you skip the test?

I did not do anything about #385, the error is still there (look at VTK 6.3 and VTK 7)
https://travis-ci.org/enthought/mayavi/jobs/128037832#L1208

Member

kitchoi commented May 5, 2016

How is this related to issue #385 ? did you skip the test?

I did not do anything about #385, the error is still there (look at VTK 6.3 and VTK 7)
https://travis-ci.org/enthought/mayavi/jobs/128037832#L1208

tvtk/tests/test_tvtk.py
+ def test_spider_plot_actor(self):
+ """ Test SpiderPlotActor Get/SetAxisLabel works
+ """
+ actor = tvtk.SpiderPlotActor()

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 12, 2016

Member

It would be nice if there were a comment as to why this object is tested that would be helpful.

@prabhuramachandran

prabhuramachandran May 12, 2016

Member

It would be nice if there were a comment as to why this object is tested that would be helpful.

@kitchoi

This comment has been minimized.

Show comment
Hide comment
@kitchoi

kitchoi May 12, 2016

Member

Note to self
TODO:

  • Test with Python 3
Member

kitchoi commented May 12, 2016

Note to self
TODO:

  • Test with Python 3
tvtk/wrapper_gen.py
+ return 'traits.Array', '', kwargs
+
+ elif type_ is bool:
+ return 'traits.Bool', '', kwargs

This comment has been minimized.

@kitchoi

kitchoi May 12, 2016

Member

note to self, return 'traits.Bool', str(value), kwargs

@kitchoi

kitchoi May 12, 2016

Member

note to self, return 'traits.Bool', str(value), kwargs

+
+ if vtk_attr_name != 'PropColorValue':
+ raise RuntimeError("Not sure why you ask for me! "
+ "I only deal with PropColorValue. Panicking.")

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Whoa, why is this needed?

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Whoa, why is this needed?

This comment has been minimized.

@kitchoi

kitchoi May 13, 2016

Member

This is to prevent this method being used with vtk_attr_name other than PropColorValue. This may be unnecessary given the method is private and is used internally.

@kitchoi

kitchoi May 13, 2016

Member

This is to prevent this method being used with vtk_attr_name other than PropColorValue. This may be unnecessary given the method is private and is used internally.

+ def _write_image_reader2_header_size(self, klass, out, vtk_attr_name):
+
+ if vtk_attr_name != 'HeaderSize':
+ raise RuntimeError("Not sure why you ask for me! "

This comment has been minimized.

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Why is this needed?

@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Why is this needed?

@prabhuramachandran

This comment has been minimized.

Show comment
Hide comment
@prabhuramachandran

prabhuramachandran May 13, 2016

Member

Thanks for this, it looks nice and the code looks a lot cleaner than before! I will merge this for now.

Member

prabhuramachandran commented May 13, 2016

Thanks for this, it looks nice and the code looks a lot cleaner than before! I will merge this for now.

@prabhuramachandran prabhuramachandran merged commit 904204d into master May 13, 2016

4 of 5 checks passed

codecov/project 45.53% (-<.01%) compared to 10f5db9
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kitchoi kitchoi deleted the fix/357-360-tvtk-with-undefined-traits branch Jun 29, 2016

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