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 491: updates recording to handle individual cells as PopulationView #492

Conversation

appukuttan-shailesh
Copy link
Contributor

This attempts to fix issue #491.
The recording is now updated to handle individual cells as PopulationView.
PopulationView class is accordingly updated to include an 'annotations' attribute.

@apdavison : Do let me know if this resolves the problem. Preliminary checks seem to suggest so. Also, take a look at the comments on lines 480-482 of pyNN/common/populations.py

p.s. Any thoughts on how to devise test cases for this. Is it ok to check saved output files in test cases? If so, will add these tests along with upcoming related fixes.

@@ -477,8 +477,11 @@ def write_data(self, io, variables='all', gather=True, clear=False, annotations=
file as metadata.
"""
logger.debug("Population %s is writing %s to %s [gather=%s, clear=%s]" % (self.label, variables, io, gather, clear))
# ignoring the method parameter 'annotations', and using class attribute 'self.annotations'
# seems ok as this method parameter does not seem to be used within pynn currently.
# TODO: decide whether this method parameter needs to be retained.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apdavison : Does this method require to accept 'annotations' as an input parameter? From a quick grep in the pynn directory, it appeared as if none of the function calls pass this parameter to the method.

@appukuttan-shailesh
Copy link
Contributor Author

@apdavison : Is this a Travis config issue? Errors such as:

ImportError: No module named nest

showing up.

if isinstance(source, (IDMixin)):
source = source.as_view()
else:
# not sure when this would be used; retaining earlier code
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apdavison : do you see any scenario where this would be used? should we remove it?

@appukuttan-shailesh
Copy link
Contributor Author

PR Closed; Merged with PR #494.

@appukuttan-shailesh appukuttan-shailesh deleted the issue_491 branch June 29, 2017 09:15
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.

None yet

1 participant