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 Issue #3337 #3338

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aserenko
Copy link
Contributor

The reason of Issue #3337 seems to be just forgotten parentheses.

Previously, one_to_one_cspec was a boolean
if isinstance(conn_spec, dict),
and a string otherwise.
This was obviously a mistake.
Note that everywhere further in _process_input_nodes()
one_to_one_cspec is always used within an if clause,
thus expecting it to be boolean.
@aserenko aserenko force-pushed the fix_unique_ids_at_node_creation_bug branch from 3869b56 to e81ce39 Compare October 17, 2024 17:04
@gtrensch gtrensch 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) T: Bug Wrong statements in the code or documentation and removed T: Maintenance Work to keep up the quality of the code and documentation. labels Oct 18, 2024
@gtrensch gtrensch self-requested a review October 18, 2024 07:28
Copy link
Contributor

@gtrensch gtrensch left a comment

Choose a reason for hiding this comment

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

Joint review. Looks good to us. Thanks for your contribution! 👍

@terhorstd
Copy link
Contributor

Hi Alex, I fixed the formatting for Black. This is automatically done if you start to use the pre-commit setup which was recently added. Have a look here, it really makes development easier.

@terhorstd
Copy link
Contributor

The unrelated warnings of pylint are handled in #3339 .

@terhorstd terhorstd added S: High Should be handled next and removed S: Normal Handle this with default priority labels Oct 18, 2024
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: In progress
Development

Successfully merging this pull request may close these issues.

"Non-unique IDs" error raised incorrectly when creating nodes not from a NodeCollection
3 participants