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

vcs.Gfb.rename() seems to destroy the object that it's renaming #2116

Closed
ghost opened this Issue Aug 31, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@ghost

ghost commented Aug 31, 2016

Here's a session example that I'm running in ipython now:

>>> b = vcs.createboxfill()
>>> b
<vcs.boxfill.Gfb at 0x1021f5ec0>
>>> b.name
'__boxfill_419032409050676'
>>> b.rename("test")
>>> vcs.elements['boxfill']['test']
<vcs.boxfill.Gfb at 0x1021f5ec0>
>>> vcs.show('boxfill')
*******************Boxfill Names List**********************
a_boxfill            a_lambert_boxfill    a_mollweide_boxfill 
a_polar_boxfill      a_robinson_boxfill   default             
polar                quick                robinson            
test                
*******************End Boxfill Names List**********************
>>> new_b = vcs.getboxfill('test')
>>> new_b
<vcs.boxfill.Gfb at 0x1021f5ec0>
>>> new_b.name
'__boxfill_419032409050676'
# assume we have a cdms2 variable named 's', and a Canvas named 'a'
>>> a.boxfill(s,b)
vcsError: Error source boxfill object (__boxfill_419032409050676) does not exist!

As we can see from the example, this leaves behind a reference to the renamed boxfill in the names list while making that boxfill unusable, because it is deleted.

I think the issue is that we should just be creating a new boxfill here, instead of using vcs.elements to re-assign things:
gfb_rename

Not sure if this is a widespread problem (affecting more than just Gfb).

@mattben mattben added the VCS label Aug 31, 2016

@mattben mattben added this to the 3.0 milestone Aug 31, 2016

@ghost

This comment has been minimized.

ghost commented Sep 19, 2016

@doutriaux1 wasn't sure if you'd seen this yet.
Forgot to tag you when I submitted it.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Sep 19, 2016

@embrown I didn't see it but i think I remember you talking to me about it.

I'll take a look.

@ghost

This comment has been minimized.

ghost commented Sep 19, 2016

@doutriaux1 I think I solved this problem. Want me to make a test and submit the new rename() code with a PR?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Sep 20, 2016

sure, but on the vcs repo please. I opened an issue for this there at: CDAT/vcs#2

@ghost

This comment has been minimized.

ghost commented Nov 17, 2016

@doutriaux1 should we close this? It was fixed in vcs.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Nov 17, 2016

@embrown please re-open if I have the wrong commit
fixed by: https://github.com/embrown/vcs/commit/b76a435b9b74b45b247c1cc4c5a1628b5a75c98f

@doutriaux1 doutriaux1 closed this Nov 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment