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.

Sign up for GitHub

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

Jump to bottom

Fix vector plots #1560

Merged
merged 9 commits into from
Sep 24, 2015
Merged

Fix vector plots #1560

merged 9 commits into from
Sep 24, 2015

Conversation

aashish24
Copy link
Contributor

WIP: I have observed tests failing on my machine but the difference seems small. I just wanted to see how they run on buildbot machine.

@aashish24
Copy link
Contributor Author

Ref: #653

@aashish24 aashish24 mentioned this pull request Sep 21, 2015
Merged
@doutriaux1
Copy link
Contributor

@aashish24 still lots of failures here. What changed?

@aashish24
Copy link
Contributor Author

@doutriaux1 thats puzzling me. On my machine, I am getting only vector tests failure. Although I am using the patched VTK

@aashish24 aashish24 force-pushed the fix_vector_plots branch 5 times, most recently from 3f97702 to 4589d40 Compare September 22, 2015 06:05
@aashish24
Copy link
Contributor Author

@doutriaux1 still the same. For some reason, I am not seeing these failing tests on my machine.

sankhesh
sankhesh reviewed Sep 23, 2015
View reviewed changes
Packages/vcs/Lib/vcsvtk/vectorpipeline.py
@@ -27,14 +30,45 @@ def plot(self, data1, data2, tmpl, grid, transform):
data1 = self._context().trimData2D(data1)
data2 = self._context().trimData2D(data2)

scale = 1.0

lat = data1.getLatitude()[:]
Copy link
Contributor

Choose a reason for hiding this comment

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

data1 could be NoneType

vcs_no_xtra_elts fails because of that. (https://open.cdash.org/testDetails.php?test=375905204&build=4024522)

@aashish24
Copy link
Contributor Author

@doutriaux1 this branch should be good to now. The problem (failing tests) was with VTK bug. I will fix the doWrap method in another PR.

@aashish24
Copy link
Contributor Author

Goes with CDAT/uvcdat-testdata#67

@doutriaux1
Copy link
Contributor

@aashish24 waiting for the bots to be done to confirm merge. Thanks for pushing this in.

@doutriaux1
Copy link
Contributor

@aashish24 4 failures on oceanonly, but I think it is because master was merged in by bots.

@aashish24
Copy link
Contributor Author

Right. there was an issue with the test-data. The only real test failing is test_no_extra_elements

@aashish24 aashish24 force-pushed the fix_vector_plots branch 2 times, most recently from 8693236 to 6e386fa Compare September 24, 2015 05:53
@aashish24
Copy link
Contributor Author

@doutriaux1 most of the tests are passing now. The multiple format one and label background one is failing for some other reason.

@aashish24
Copy link
Contributor Author

@doutriaux1 this branch is looking good now. Not sure why the label and the click test is failing.

https://open.cdash.org/testDetails.php?test=376700167&build=4028520

@durack1
Copy link
Member

durack1 commented Sep 24, 2015

@aashish24 the reason is due to problems with the test image, so compare the baseline:
test_vcs_click_info
With the test output:
test
You'll see that there is bleeding of color outside the box, and the y-axis labels in particular are offset, look at 90N.. There are also problems with the x-axis labels and their placement too..

@durack1 durack1 mentioned this pull request Sep 24, 2015
Merged
@aashish24
Copy link
Contributor Author

@doutriaux1 could it be caused by my changes? thanks @durack1

@durack1
Copy link
Member

durack1 commented Sep 24, 2015

@aashish24 I've been seeing this issue in a number of PRs #1540, #1560, #1557 - and it only seems to be a problem on garant - so an OS-specific or machine-specific issue?

@durack1
Copy link
Member

durack1 commented Sep 24, 2015

@aashish24 @doutriaux1 I'm wondering if garant is just not syncing correctly with the updated source and as it's behind we're getting these failures - looks like the case with #1564..

@jbeezley
Copy link
Contributor

It doesn't merge in master like the others. Is that the issue?

@durack1
Copy link
Member

durack1 commented Sep 24, 2015

@jbeezley it seems that in particular with #1564 FFmpeg issues are only occurring on garant, whereas everything else passes.. So like it fails to pull across all the PR changes or something? Dunno?

@aashish24
Copy link
Contributor Author

thanks @durack1 for the information. In that case, I think this branch looks good to me. Can someone review it @durack1 @sankhesh @jbeezley @chaosphere2112

sankhesh
sankhesh reviewed Sep 24, 2015
View reviewed changes
testing/vcs/test_vcs_vectors_robinson.py
@@ -0,0 +1,24 @@
import vcs, cdms2, numpy, os, sys
Copy link
Contributor

Choose a reason for hiding this comment

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

This file doesn't comply with PEP8 style.

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 but I don't see flake8 test failing. what I am missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't check the test scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. Thanks @jbeezley. I will push a fix to follow the PEP8 stype in my next branch as I need to take care of doWrap as well.

doutriaux1 added a commit that referenced this pull request Sep 24, 2015
@doutriaux1
Merge pull request #1560 from UV-CDAT/fix_vector_plots
7241b33 
Fix vector plots
@doutriaux1 doutriaux1 merged commit 7241b33 into master Sep 24, 2015
@aashish24
Copy link
Contributor Author

thanks @doutriaux1

@aashish24 aashish24 mentioned this pull request Oct 9, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aashish24 @doutriaux1 @durack1 @jbeezley @sankhesh