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

CSD-316 new function to convert networkx to GraphData object #1

Merged
merged 13 commits into from
Dec 12, 2024

Conversation

kibidi
Copy link

@kibidi kibidi commented Dec 2, 2024

Added a function that converts a networkx data object to the required GraphData object. Also added a test and one of the dependent functions broke

@diegoabt diegoabt self-requested a review December 2, 2024 17:12
@diegoabt diegoabt assigned diegoabt and kibidi and unassigned diegoabt Dec 2, 2024
Copy link
Collaborator

@diegoabt diegoabt left a comment

Choose a reason for hiding this comment

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

Great work! Thank you for proposing this function; very useful! I left a few suggestions that would be nice to implement before adding this feature into the package. I'll be happy to discuss this further. 😃

probinet/input/loader.py Outdated Show resolved Hide resolved
probinet/input/loader.py Outdated Show resolved Hide resolved
probinet/input/loader.py Outdated Show resolved Hide resolved
probinet/input/loader.py Outdated Show resolved Hide resolved
tests/test_input.py Show resolved Hide resolved
tests/test_input.py Outdated Show resolved Hide resolved
tests/test_input.py Show resolved Hide resolved
@diegoabt
Copy link
Collaborator

diegoabt commented Dec 3, 2024

@kibidi , you might need to sync your fork before working on the review. We've added a couple of new features into the dev branch since you opened the PR.

Fix command
Fix command

Add any branch to trigger action
@kibidi kibidi force-pushed the issue/CSD-316-nx-loader branch from 83734e6 to a2144b4 Compare December 4, 2024 10:27
kibidi and others added 5 commits December 4, 2024 12:23
Co-authored-by: Diego Baptista Theuerkauf <34717973+diegoabt@users.noreply.github.com>
Copy link
Member

@jcpassy jcpassy left a comment

Choose a reason for hiding this comment

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

Thanks a lot @kibidi for your contribution. :)
My comments are only minor ones and related to style and standards. Great job! 👍

probinet/input/loader.py Show resolved Hide resolved
probinet/input/loader.py Outdated Show resolved Hide resolved
probinet/input/loader.py Outdated Show resolved Hide resolved
tests/test_input.py Show resolved Hide resolved
tests/test_input.py Outdated Show resolved Hide resolved
@kibidi
Copy link
Author

kibidi commented Dec 12, 2024

@jcpassy, thank you very much! It was fun!

@diegoabt diegoabt merged commit 047cc25 into MPI-IS:develop Dec 12, 2024
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.

3 participants