Skip to content

121 missing value opacity - #123

Merged
doutriaux1 merged 2 commits into
CDAT:masterfrom
sankhesh:121_missing_value_opacity
Jan 30, 2017
Merged

121 missing value opacity#123
doutriaux1 merged 2 commits into
CDAT:masterfrom
sankhesh:121_missing_value_opacity

Conversation

@sankhesh

Copy link
Copy Markdown
Contributor

Fixes #121

@durack1

durack1 commented Jan 26, 2017

Copy link
Copy Markdown
Member

@doutriaux1 if this can be added to the patterns branch and whacked into a nightly I'll give it a run with my failing script

@durack1

durack1 commented Jan 26, 2017

Copy link
Copy Markdown
Member

@sankhesh thanks for looking at this, I'm looking forward to getting boxfill working with opacities

@sankhesh

Copy link
Copy Markdown
Contributor Author

@danlipsa Please review

@sankhesh

Copy link
Copy Markdown
Contributor Author

@durack1 Sure thing.

@doutriaux1

Copy link
Copy Markdown
Contributor

@sankhesh thanks! @danlipsa I will let you double check the removehiddencellpoint section. It looks right to me, but I would feel better if you review it as well. Once you're happy with it please merge in master.

Comment thread vcs/vcs2vtk.py Outdated
pass
else:
removeHiddenPoints(grid)
removeHiddenPointsOrCells(grid, cellData)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing the if statement.

Comment thread vcs/vcs2vtk.py Outdated
# We remove the cells in this case.
if (vg.GetExtentType() == vtk.VTK_PIECES_EXTENT):
removeHiddenPoints(vg)
removeHiddenPointsOrCells(vg, False)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to:removeHiddenPointsOrCells(vg, cellData=False)

This change makes sure that the hidden cells from the masked dataset are
removed before mapping. If they are not removed, they will be visible
when the opacity of the mask is less than 100.
@sankhesh
sankhesh force-pushed the 121_missing_value_opacity branch from 1ca3998 to f6603cd Compare January 26, 2017 22:13
@sankhesh

Copy link
Copy Markdown
Contributor Author

Thanks @danlipsa.

Just pushed the suggested changes.

@durack1

durack1 commented Jan 26, 2017

Copy link
Copy Markdown
Member

@sankhesh it would be great to add a test for this, just so future changes (and similar weird behaviours) can be caught by the test suite rather than merged

@sankhesh

Copy link
Copy Markdown
Contributor Author

Good idea @durack1. I'll add it.

@sankhesh

Copy link
Copy Markdown
Contributor Author

@doutriaux1 @durack1 Okay to merge?

@durack1

durack1 commented Jan 30, 2017

Copy link
Copy Markdown
Member

@doutriaux1 are we good to go here? I would really like to get these graphics moving

@doutriaux1
doutriaux1 merged commit 72b4495 into CDAT:master Jan 30, 2017
@sankhesh
sankhesh deleted the 121_missing_value_opacity branch January 30, 2017 17:02
@durack1

durack1 commented Feb 5, 2017

Copy link
Copy Markdown
Member

@doutriaux1 just checking https://anaconda.org/uvcdat/vcs-nox/files should a new nightly appear automagically, or can you trigger one?

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.

4 participants