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

Return empty dictionary if get() is called on empty SynapseCollection #2332

Merged
merged 3 commits into from
Mar 18, 2022

Conversation

stinebuu
Copy link
Contributor

@stinebuu stinebuu commented Mar 8, 2022

This fixes #2318.

@stinebuu stinebuu added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) good first issue Good for newcomers labels Mar 8, 2022
@stinebuu stinebuu added this to In progress in PyNEST via automation Mar 8, 2022
@stinebuu stinebuu moved this from In progress to Review in PyNEST Mar 8, 2022
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Could you also add a test of conns.get() on an empty SynapseCollection with specified keys (the case that should return ())?

pynest/nest/lib/hl_api_types.py Outdated Show resolved Hide resolved
stinebuu and others added 2 commits March 8, 2022 15:54
Co-authored-by: Håkon Bakke Mørk <hakon.mork@nmbu.no>
@stinebuu stinebuu requested a review from hakonsbm March 8, 2022 15:11
Copy link
Contributor

@hakonsbm hakonsbm left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@med-ayssar med-ayssar left a comment

Choose a reason for hiding this comment

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

Otherwise, it looks good to me.


nrns = nest.Create('iaf_psc_alpha', 2)
nest.Connect(nrns, nrns, 'one_to_one')
conns = nest.GetConnections()
self.assertEqual(len(conns), 2)

nest.ResetKernel()
self.assertEqual(conns.get(), ())
self.assertEqual(conns.get(), {})
conns.set(weight=10.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is not resetting the kernel equivalent to adding or deleting connections, and in that case we should get a warning from the ConnectionManager stating a similar info as the following: ConnectionManager [Warning]: New connections created, connection descriptors previously obtained using 'GetConnections' are now invalid. In that case, the user should know that setting the weight in conn object won't be a valid operation, or maybe throw an error that the weight isn't in the connection dictionary anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

@med-ayssar Could you make an issue for that?

@hakonsbm hakonsbm merged commit ac1f40b into nest:master Mar 18, 2022
PyNEST automation moved this from Review to Done Mar 18, 2022
@stinebuu stinebuu deleted the empty_synapsecollection branch March 22, 2022 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
PyNEST
  
Done
Development

Successfully merging this pull request may close these issues.

Inconsistent returned type in nest.GetConnections().get()
3 participants