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

Fixes #720 Issues related to current recording from devices #733

Merged
merged 7 commits into from May 29, 2017

Conversation

@appukuttan-shailesh
Copy link
Contributor

appukuttan-shailesh commented May 25, 2017

Implements the following (as discussed in #720 and #663 ):

  • Setting the recordables: GetDefaults('..._generator')['recordables']
  • Appropriate updates to unit test to include above
  • For noise generator, set current = 0, when no target neuron connected
  • For noise generator, keep the current value constant for the duration of noise_generator's dt
  • (solutions for above discussed in #663)
@appukuttan-shailesh appukuttan-shailesh changed the title Bugfix get recordables Fixes #720 May 25, 2017
Copy link
Contributor

heplesser left a comment

This looks fine, except for a little syntax cleanup in the new tests.

try:
val = nest.GetDefaults('step_current_generator')['recordables'][0]
assert val == 'I', \
"Incorrect recordables ( %r ) for step current generator" % val

This comment has been minimized.

@heplesser

heplesser May 26, 2017 Contributor

Please use

self.assertEqual(val, 'I', 'Incorrect recordables ({}) for step current generator'.format(val))

also for the tests below.

This comment has been minimized.

@appukuttan-shailesh

appukuttan-shailesh May 26, 2017 Author Contributor

Just to confirm, I would be replacing

try:
        val = nest.GetDefaults('step_current_generator')['recordables'][0]
        assert val == 'I', \
                "Incorrect recordables ( %r ) for step current generator" % val
except:
        raise

with

val = nest.GetDefaults('step_current_generator')['recordables'][0]
self.assertEqual(val, 'I', 'Incorrect recordables ({}) for step current generator'.format(val))

for each of the sub-tests

This comment has been minimized.

@heplesser

heplesser May 26, 2017 Contributor

Correct.

@heplesser heplesser changed the title Fixes #720 Fixes #720 Issues related to current recording from devices May 26, 2017
@heplesser
Copy link
Contributor

heplesser commented May 26, 2017

@appukuttan-shailesh Could you pep8-ify the test file? Some lines are too long and there is trailing whitespace.

@appukuttan-shailesh
Copy link
Contributor Author

appukuttan-shailesh commented May 26, 2017

Done.... looks good to go.

Copy link
Collaborator

gtrensch left a comment

Many thanks. Looks good to me.

@heplesser
Copy link
Contributor

heplesser commented May 29, 2017

This merge fixes current recording from noise generator and adds the recordables entry to other generators that support current recording.

@heplesser heplesser merged commit 0e21996 into nest:master May 29, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.