Apply has_attribute to the output in VTKFileReader #315

Merged
merged 2 commits into from Mar 28, 2016

Conversation

Projects
None yet
2 participants
@stefanoborini
Contributor

stefanoborini commented Mar 16, 2016

Fixes failing tests in simphony-mayavi due to incorrect application of assign_attribute
to a PolyData dataset.

Apply has_attribute to the output in VTKFileReader
Fixes failing tests in simphony-mayavi due to incorrect application of assign_attribute
to a PolyData dataset.

@stefanoborini stefanoborini referenced this pull request in simphony/simphony-mayavi Mar 16, 2016

Open

VTK Error for test_load_blowGeom_vtk and test_load_vtk_vtk #112

mayavi/sources/vtk_file_reader.py
- outputs[0] = aa.output
+ # FIXME: currently handling only one output (the first one) with
+ # assign attributes.
+ for index, output in enumerate(outputs):

This comment has been minimized.

@prabhuramachandran

prabhuramachandran Mar 17, 2016

Member

I am not sure this will do as you expect, does your test case really have multiple outputs? Multiple outputs will not work in Mayavi as most of our code implicitly assumes one output. This should be revisited perhaps in the future.

@prabhuramachandran

prabhuramachandran Mar 17, 2016

Member

I am not sure this will do as you expect, does your test case really have multiple outputs? Multiple outputs will not work in Mayavi as most of our code implicitly assumes one output. This should be revisited perhaps in the future.

This comment has been minimized.

@stefanoborini

stefanoborini Mar 17, 2016

Contributor

It only has one, but then probably it's just missing a break, or I should simply remove the for loop altogether. The critical point is the has_attributes part. If you say it's a requirement that we only have one output, then it's certainly the case. I revise it.

@stefanoborini

stefanoborini Mar 17, 2016

Contributor

It only has one, but then probably it's just missing a break, or I should simply remove the for loop altogether. The critical point is the has_attributes part. If you say it's a requirement that we only have one output, then it's certainly the case. I revise it.

@prabhuramachandran

This comment has been minimized.

Show comment
Hide comment
@prabhuramachandran

prabhuramachandran Mar 28, 2016

Member

LGTM, merging.

Member

prabhuramachandran commented Mar 28, 2016

LGTM, merging.

@prabhuramachandran prabhuramachandran merged commit 94c8e88 into master Mar 28, 2016

5 checks passed

codecov/project 45.37% (target 40.00%)
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

@prabhuramachandran prabhuramachandran deleted the feature/assign-attributes-applied-to-polydata-0316-1820 branch Mar 28, 2016

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