Skip to content

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

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

Merged
merged 3 commits into from
Nov 9, 2015
Merged

Conversation

chaosphere2112
Copy link
Contributor

Resolves issue #1620

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

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

@chaosphere2112
Copy link
Contributor Author

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

@doutriaux1
Copy link
Contributor

done at: #1622

@doutriaux1
Copy link
Contributor

@chaosphere2112 it seems to have broken: vcs_verify_remove_objects

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

Awesome! Thanks @chaosphere2112

@chaosphere2112
Copy link
Contributor Author

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

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

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

@aashish24
Copy link
Contributor

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

👍

@chaosphere2112
Copy link
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