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

BUG #26: If masking reduces the bounds of a dataset, plots look incom… #27

Merged
merged 1 commit into from Nov 2, 2016

Conversation

danlipsa
Copy link
Contributor

…plete.

We use 'vtk_dataset_bounds_no_mask', the dataset bounds before masking
instead of the dataset bounds for fitToViewport.

@danlipsa
Copy link
Contributor Author

danlipsa commented Oct 26, 2016

Data and baselines:
CDAT/uvcdat-testdata#159
Tests
CDAT/cdat#2134

@danlipsa
Copy link
Contributor Author

@doutriaux1 @aashish24 @sankhesh Please review.

@danlipsa
Copy link
Contributor Author

danlipsa commented Oct 26, 2016

@doutriaux1 It turned out that what I thought was a bug in VTK was a feature. 😄 So no VTK update is required for this.

Copy link
Contributor

@sankhesh sankhesh left a comment

Choose a reason for hiding this comment

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

LGTM except the debug variable value

@@ -14,6 +14,25 @@
import sys
import numbers

_DEBUG_VTK = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't _DEBUG_VTK be set to False?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sankhesh Thanks, I will remove the debug code.

…plete.

We use 'vtk_dataset_bounds_no_mask', the dataset bounds before masking
instead of the dataset bounds for fitToViewport.
@danlipsa
Copy link
Contributor Author

@doutriaux1 @aashish24 Ready to merge?

@doutriaux1
Copy link
Contributor

i need to run the tests and look at it, sorry I'm on another project today.

@aashish24
Copy link
Contributor

@danlipsa looks reasonable to me.

@@ -596,8 +598,9 @@ def plot(self, data1, data2, template, gtype, gname, bg, *args, **kargs):
ren = kargs["renderer"]

vtk_backend_grid = kargs.get("vtk_backend_grid", None)
vtk_dataset_bounds_no_mask = kargs.get("vtk_dataset_bounds_no_mask", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

None is returned by default but I am okay with more specifics.

@aashish24
Copy link
Contributor

@sankhesh @doutriaux1 can we merge this branch?

@doutriaux1
Copy link
Contributor

let me run it, sorry about that.

@doutriaux1 doutriaux1 merged commit cf99af4 into master Nov 2, 2016
@doutriaux1 doutriaux1 deleted the masked-dataset-bounds branch November 2, 2016 22:59
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.

None yet

4 participants