Skip to content
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

Merged
merged 3 commits into from Jul 13, 2015

Conversation

doutriaux1
Copy link
Contributor

and removes them at clear time
fix #1424

and removes them at clear time
fix #1424
@doutriaux1
Copy link
Contributor Author

@aashish24 @dlonie @sankhesh please review

@durack1
Copy link
Member

durack1 commented Jul 8, 2015

@doutriaux1 is this some kind of alternate very efficient language which uses every 3rd letter?

@doutriaux1
Copy link
Contributor Author

@durack1 what can I say? My brain goes so fast that my physical abilities just can't keep up 😜

@durack1
Copy link
Member

durack1 commented Jul 8, 2015

@doutriaux1 do you want me to run the code indicated in #1424 to test this over a large number of loops?

@doutriaux1
Copy link
Contributor Author

@aashish24 ?

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

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!

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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...

Copy link
Contributor Author

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.

@aashish24
Copy link
Contributor

@doutriaux1 let's merge it then. If are planning to take care of vcs style issues soon. 👍

aashish24 added a commit that referenced this pull request Jul 13, 2015
keep track of temporry elets added hen plotting
@aashish24 aashish24 merged commit 5f89b08 into master Jul 13, 2015
@aashish24 aashish24 deleted the issue_1424_elets_not_cleared branch July 13, 2015 13:03
@doutriaux1
Copy link
Contributor Author

Thanks @aashish24 as soon as I reviewed and merged @dlonie PR I will flake8 it.

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.

vcs.plot(iso) creates text object that are not removed after clear
4 participants