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

Issue 1234 boxfill anim broken #1236

Merged
merged 4 commits into from Apr 28, 2015
Merged

Conversation

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Apr 27, 2015

@aashish24 @dlonie @chaosphere2112 goes with:
CDAT/uvcdat-testdata#37

doutriaux1 added 3 commits Apr 27, 2015
add to let update know to use CellData rather than PointData
code was in place but was returning False rather than True in this instance
fix #1234
still need to add tests
add to let update know to use CellData rather than PointData
code was in place but was returning False rather than True in this instance
fix #1234
still need to add tests
@@ -1153,7 +1153,7 @@ def plot2D(self,data1,data2,tmpl,gm,vtk_backend_grid=None,vtk_backend_geo=None):
else:
lmx= levs[-1]
mapper.SetScalarRange(lmn,lmx)
returned["vtk_backend_luts"]=[[lut,[lmn,lmx,False]],]
returned["vtk_backend_luts"]=[[lut,[lmn,lmx,True]],]
Copy link
Contributor

@allisonvacanti allisonvacanti Apr 28, 2015

These map entries really need some documentation. I have no idea what this change should do.

Copy link
Contributor Author

@doutriaux1 doutriaux1 Apr 28, 2015

yes but aren't we changing all of this anyway once we refactor?

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Apr 28, 2015

Building/testing now.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Apr 28, 2015

hey guys.. not picking on anyone but please use better short messages for the commit.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Apr 28, 2015

@dlonie I am assuming that you will have some feedback on it.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Apr 28, 2015

Yep, still building. Assuming all goes well this can go in -- we can work on documenting that map when the animation code gets hit by the refactoring sweep.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Apr 28, 2015

👍 in general we should document our code as much as possible atleast when something is not trivial or error prone. In my document, I will make it a requirement.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Apr 28, 2015

LGTM, tests pass.

allisonvacanti pushed a commit that referenced this issue Apr 28, 2015
@allisonvacanti allisonvacanti merged commit 29a1e60 into master Apr 28, 2015
0 of 3 checks passed
@allisonvacanti allisonvacanti deleted the issue_1234_boxfill_anim_broken branch Apr 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants