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: adapt code so it works with different versions of networkx #1137

Merged
merged 3 commits into from
Sep 21, 2023

Conversation

celprov
Copy link
Collaborator

@celprov celprov commented Sep 8, 2023

fix: Resolve dependency issue with newer version of networkx

Closes #1136

@celprov celprov marked this pull request as ready for review September 11, 2023 20:27
@celprov
Copy link
Collaborator Author

celprov commented Sep 11, 2023

@esavary @oesteban GitHub action build (3.11, pip==21.2) complains there is no space left on the device. Is there something that should be done at the organization level to solve this issue?

Error: uld consider upgrading via the '/tmp/install_wheel/bin/python -m pip install --upgrade p2023-09-11T20:22:28.8880057Z ##[error]No space left on device : '/home/runner/runners/2.308.0/_diag/pages/97591c64-e054-4d6f-89f6-8ddafade3bbf_e054ce87-d82e-512d-3ab3-06d2fef876a4_1.log'

PS as it also fails on PR #1140 it seems more likely the problem is GitHub action itself than my code.

try:
self.refidx[self.refidx[:, jobid].nonzero()[0], jobid] = 0
except NotImplementedError:
self.refidx[self.refidx[:, [jobid]].nonzero()[0], jobid] = 0
Copy link
Member

Choose a reason for hiding this comment

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

what is this addressing? Is there an issue open for this failure condition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I changed l. 351, MRIQC was running into a NotImplementedError because they did not implement the feature for 1D arrays. Sorry, I cannot find again the exact error message so I don't remember which software was issuing the error. But, the error message suggested this workaround.

@effigies
Copy link
Member

Since this looks like mostly a copy from nipype, it might be best just to copy what we did in nipype: nipy/nipype@443492e

@pep8speaks
Copy link

pep8speaks commented Sep 19, 2023

Hello @celprov, Thank you for updating!

Line 183:80: E501 line too long (89 > 79 characters)
Line 290:80: E501 line too long (89 > 79 characters)
Line 291:80: E501 line too long (95 > 79 characters)
Line 308:80: E501 line too long (95 > 79 characters)
Line 345:80: E501 line too long (80 > 79 characters)
Line 348:80: E501 line too long (85 > 79 characters)
Line 503:80: E501 line too long (93 > 79 characters)

To test for issues locally, pip install flake8 and then run flake8 niworkflows.

Comment last updated at 2023-09-19 17:56:05 UTC

@celprov celprov force-pushed the fix/networkx branch 2 times, most recently from e8b0c85 to cf681a2 Compare September 19, 2023 17:53
@celprov
Copy link
Collaborator Author

celprov commented Sep 19, 2023

Since this looks like mostly a copy from nipype, it might be best just to copy what we did in nipype: nipy/nipype@443492e

Thanks @effigies for the suggestion. I implemented it in the last commit.

@oesteban oesteban merged commit 83a66e0 into nipreps:master Sep 21, 2023
13 of 14 checks passed
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.

Dependency issue with networkx
4 participants