-
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
keep track of temporry elets added hen plotting #1449
Conversation
and removes them at clear time fix #1424
@aashish24 @dlonie @sankhesh please review |
@doutriaux1 is this some kind of alternate very efficient language which uses every 3rd letter? |
@durack1 what can I say? My brain goes so fast that my physical abilities just can't keep up 😜 |
@doutriaux1 do you want me to run the code indicated in #1424 to test this over a large number of loops? |
@@ -2679,11 +2679,25 @@ def plot_filledcontinents(self,slab,template_name,g_type,g_name,bg,ratio): | |||
except Exception,err: | |||
print err | |||
|
|||
def __new_elts(self,original,new): |
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.
These wouldn't pass the style checks. Let's try to use pep8 style in new commits (spaces between arguments/list items, 4 space indents, etc) -- less work to do later!
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.
please approve, I'll do the pep8 anyway and autopep8 does this... No time for this right now. Thanks.
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.
@doutriaux1 if this is urgent, I would suggest you pass the branch name to the folks that need it (or create staging). I think it would be better if we take care of the code at the review level.
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.
let's let it rot here then... I just can't get to it now, it fixes memory growth issue. I still need to run pep8 fix on vcs anyway, I don't want to it twice...
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.
when I do the vcs pep8, then I will rebase this on top of it and do the pep8 on this too. You guys can approve it then. That's fine with me.
@doutriaux1 let's merge it then. If are planning to take care of vcs style issues soon. 👍 |
keep track of temporry elets added hen plotting
Thanks @aashish24 as soon as I reviewed and merged @dlonie PR I will flake8 it. |
and removes them at clear time
fix #1424