Skip to content

Fix vector plots#1560

Merged
doutriaux1 merged 9 commits intomasterfrom
fix_vector_plots
Sep 24, 2015
Merged

Fix vector plots#1560
doutriaux1 merged 9 commits intomasterfrom
fix_vector_plots

Conversation

@aashish24
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Ref: #653

@doutriaux1
Copy link
Copy Markdown
Contributor

@aashish24 still lots of failures here. What changed?

@aashish24
Copy link
Copy Markdown
Contributor Author

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

@aashish24
Copy link
Copy Markdown
Contributor Author

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

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.

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
Copy Markdown
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
Copy Markdown
Contributor Author

Goes with CDAT/uvcdat-testdata#67

@doutriaux1
Copy link
Copy Markdown
Contributor

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

@doutriaux1
Copy link
Copy Markdown
Contributor

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

@aashish24
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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..

@aashish24
Copy link
Copy Markdown
Contributor Author

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

@durack1
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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

@durack1
Copy link
Copy Markdown
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
Copy Markdown
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

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.

This file doesn't comply with PEP8 style.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 doutriaux1 merged commit 7241b33 into master Sep 24, 2015
@aashish24
Copy link
Copy Markdown
Contributor Author

thanks @doutriaux1

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.

5 participants