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

Fix corner case for node collections #3214

Merged
merged 3 commits into from
Jun 4, 2024
Merged

Conversation

heplesser
Copy link
Contributor

This PR fixes #3213.

@heplesser heplesser added T: Bug Wrong statements in the code or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jun 3, 2024
@clinssen
Copy link
Contributor

clinssen commented Jun 4, 2024

I confirm that this fixes the issues we were seeing in NESTML. Thank you for the quick fix!

@otcathatsya
Copy link
Contributor

Looks good to me, although the test also passes without the fix (on master)

@clinssen
Copy link
Contributor

clinssen commented Jun 4, 2024

Looks good to me, although the test also passes without the fix (on master)

That's strange! I double-checked that the test (at least the code snippet from num_n = ... to the assert) fails for me on master and passes on this branch. Do the lines fail for you if you paste them into an ipython session, but the test passes? Or they are both passing?

@otcathatsya
Copy link
Contributor

Looks good to me, although the test also passes without the fix (on master)

That's strange! I double-checked that the test (at least the code snippet from num_n = ... to the assert) fails for me on master and passes on this branch. Do the lines fail for you if you paste them into an ipython session, but the test passes? Or they are both passing?

My bad, forgot to update my fork! One could argue that maybe it should be part of the general slicing test case but that's just nitpicking, lgtm.

@heplesser heplesser merged commit d49d1b5 into nest:master Jun 4, 2024
24 checks passed
@heplesser heplesser deleted the fix-3213 branch June 5, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Bug Wrong statements in the code or documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

GetConnections() triggers assertion for some node collections given as sources
3 participants