-
Notifications
You must be signed in to change notification settings - Fork 68
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
Docstring cleanup #2056
Docstring cleanup #2056
Conversation
@embrown OK, I've fixed this branch up so GitHub likes it more; I also fixed some code style issues. However, there are 41 lines that are longer than our limit for code style (maximum line length is 120 characters). Canvas.py
manageElements.py
To get this current version of docstring_cleanup, you should do |
@@ -1553,3 +1553,9 @@ def png_dimensions(self, path): | |||
img = reader.GetOutput() | |||
size = img.GetDimensions() | |||
return size[0], size[1] | |||
|
|||
def canvasraised(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chaosphere2112 what is this? The name almost suggest this is a query I would expect it to return True/False shouldn't this be called raiseCanvas
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the VTK implementation of this function, which previously said this:
#############################################################################
# #
# Raise VCS Canvas to the top of all its siblings. #
# #
#############################################################################
def canvasraised(self, *args):
"""
Function: canvasraised # Raise the VCS Canvas to the top
Description of Function:
This function marks a VCS Canvas as eligible to be displayed and
positions the window at the top of the stack of its siblings.
Example of Use:
a=vcs.init()
...
a.canvasraised()
"""
return apply(self.canvas.canvasraised, args)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seemed handy, so I actually implemented it.
@chaosphere2112 I have one little question. |
This reverts commit 433de3f.
@doutriaux1 I'm running the test suite on the branch just to double check, then I'll merge. |
@chaosphere2112 is this reviewed by @doutriaux1 and someone else? |
@aashish24 It's @embrown's (our intern, Ed) PR, I just cleaned it up a little. It's been reviewed by @doutriaux1 and myself. |
@aashish24 I just want to get this merged into master so Ed can start a new branch and continue working ASAP |
@aashish24 Feel free to give it a quick perusal as well 😄 |
Sounds good @chaosphere2112 I did a quick pass, and it looks good to me. |
Cleaned up version of #2055