Skip to content

Fixed issue where XvsY was using the wrong element name#1621

Merged
doutriaux1 merged 3 commits intomasterfrom
fix_1620
Nov 9, 2015
Merged

Fixed issue where XvsY was using the wrong element name#1621
doutriaux1 merged 3 commits intomasterfrom
fix_1620

Conversation

@chaosphere2112
Copy link
Copy Markdown
Contributor

Resolves issue #1620

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

Apparently this applies to all 1D plots that are done via the canvas methods. I'm going to fix them all.

@williams13
Copy link
Copy Markdown
Contributor

Thanks Sam!

From: Sam Fries <notifications@github.commailto:notifications@github.com>
Reply-To: UV-CDAT/uvcdat <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, October 19, 2015 at 10:07 AM
To: UV-CDAT/uvcdat <uvcdat@noreply.github.commailto:uvcdat@noreply.github.com>
Subject: Re: [uvcdat] Fixed issue where XvsY was using the wrong element name (#1621)

Apparently this applies to all 1D plots that are done via the canvas methods. I'm going to fix them all.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1621#issuecomment-149283583.

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

@doutriaux1 @williams13

xvsy
yxvsx
xyvsy
scatter

Looks like that's all of the ones in manageElements; should be good now?

@doutriaux1
Copy link
Copy Markdown
Contributor

Ok should we make it exit nicely in that PR or should we create a separate issue for this?

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

I think that should be in a separate issue; this one is pretty cut and dry.

@doutriaux1
Copy link
Copy Markdown
Contributor

done at: #1622

@doutriaux1
Copy link
Copy Markdown
Contributor

@chaosphere2112 it seems to have broken: vcs_verify_remove_objects

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

OK, so I tweaked how we store 1D objects in vcs.elements. They're now exclusively stored in vcs.elements["1d"]. On a name collision, it will attempt to create a new one called name_xyvsy and issue a warning that the GM will have a different name than the one provided. When retrieving name, it will call get1d(name). If that returns a graphics method, it checks the g_type property on the object, which returns the 1D type based on the attributes of the object (xyvsy has flip set to True, scatter has linewidth set to 0). If the g_type doesn't line up with what we're expecting, it'll try and retrieve name_xyvsy (so users who don't see the warning should hopefully have everything happen correctly).

isyxvsx and isxvsy will now (possibly confusingly) claim that one graphics method could be either. This is true from the graphics method standpoint; xvsy and yxvsx are only different with regards to the number of slabs passed, which means that from this layer of the codebase, they're functionally identical. This is why I updated the test_queries test.

@aashish24 @doutriaux1

@doutriaux1
Copy link
Copy Markdown
Contributor

Awesome! Thanks @chaosphere2112

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

@doutriaux1 Any idea what's up with the failed tests on crunchy?

@doutriaux1
Copy link
Copy Markdown
Contributor

it says

ERROR: In /export/doutriaux1/uvcbot/build/build/VTK/Rendering/OpenGL/vtkXOpenGLRenderWindow.cxx, line 1473
vtkXOpenGLRenderWindow (0x2e19f80): bad X server connection. DISPLAY=:5. Aborting.

probably need to restart Xvfb

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

@doutriaux1 @aashish24 So are we good to merge, then?

@aashish24
Copy link
Copy Markdown
Contributor

@chaosphere2112 looks good to me if @doutriaux1 is okay with this change.

👍

@chaosphere2112
Copy link
Copy Markdown
Contributor Author

@doutriaux1 @aashish24 ping

doutriaux1 added a commit that referenced this pull request Nov 9, 2015
Fixed issue where XvsY was using the wrong element name
@doutriaux1 doutriaux1 merged commit 2fe16b8 into master Nov 9, 2015
@doutriaux1 doutriaux1 deleted the fix_1620 branch November 9, 2015 16:41
@doutriaux1 doutriaux1 mentioned this pull request Nov 10, 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

Development

Successfully merging this pull request may close these issues.

4 participants