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

Feature renameRoutine #1136

merged 8 commits into from Mar 29, 2016

Feature renameRoutine #1136

merged 8 commits into from Mar 29, 2016


Copy link

@OliJimbo OliJimbo commented Mar 10, 2016

Added setitem to get rid of the old name from the component params
list - this does not fix the previous commit error!

OliJimbo added 2 commits Mar 9, 2016
Has functions that change name of routine to a user specified name.
Changes notebook, namespace and routines dictionary.

Will not compile script if component added after renaming.  This is
despite there being no reference in the sublist or any dictionary.
Interestingly, if the name is changed for the second time with the same
name - makeValid is invoked and e.g. fooBar_2 is returned.  There is no
visible reference to fooBar in self.user.

The error message returned when script compilation fails is:
File "/psychopy/psychopy/app/builder/components/", line 326, in
    routine = self.exp.routines[self.parentName]
KeyError: 'hello_world'

I have also included a context menu entry but this is not active yet.
Added __setitem__ to get rid of the old name from the component params
list - this does not fix the previous commit error!

This comment has been minimized.

Copy link

@peircej peircej commented on 7fc1039 Mar 10, 2016

Ah, OK
All components store an attribute self.parentName so that we can track back to which Routine they belong to. Also, I've found a one-liner y to rename the key within a dictionary:
routines['aNewName'] = routines.pop('someOldname')

Combining the two things should be roughly this (at line 2039):

self.exp.routines[oldName].name = name
self.exp.routines[name] = self.exp.routines.pop(oldName)
for comp in self.exp.routines[name]
    comp.parentName = name

This comment has been minimized.

Copy link

@jeremygray jeremygray replied Mar 10, 2016

Its good to see someone take this up and fix it! Its been an issue since psychopy#142.
The changes to psychopy/ probably can all be reverted (unless I missed something). And it would be good to run autopep8 on your new code to bring the whitespace formatting in line with the recently adopted code style for the PsychoPy project. And when the time comes, you can remove the # OLI ADDED comments -- git / github keeps track of that information automatically.

OliJimbo added 4 commits Mar 11, 2016
Copy link

@OliJimbo OliJimbo commented Mar 12, 2016

Thanks Jon and Jeremy - I had played around with pop() a few times but it seemed not to work - so I must have used it incorrectly.

I have now pepped it and the menu version is fine. I'll work on the context menu later (I'll need to add a little more code so rename only appears when routine is selected rather than the loop as well).

Added Rename to context menu.  This only appears when a routine is
selected. Could add loop renaming function if requested, but doesn’t
seem important currently.
Copy link

@OliJimbo OliJimbo commented Mar 16, 2016

Hi Jon and Jeremy,

I have now implemented a rename option in the context menu which will only appear when a routine is selected. Above, I am told that there are conflicts - but when I run git status nothing shows up so I'm not sure where the problem is!

Also - I am not sure why I edited init so I'd be happy to revert it, if I run git revert(7fc1039) will this undo everything, or just the files, like init which haven't been changed since this commit.

Many thanks


Copy link

@jeremygray jeremygray commented Mar 16, 2016

To revert the file, I am not sure what git revert will do. (I should look it up sometime!) You will get the desired result if you copy the old file and replace the new one with the old one, and make a new commit.

@peircej peircej merged commit 466cbc6 into psychopy:master Mar 29, 2016
@jeremygray jeremygray mentioned this pull request Mar 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants