Conversation
NiBabel 4 began warning that int64 images would error, and NiBabel 5 began erroring if not passed an explicit dtype or header. We don't need int64 images, just set some sensible dtypes.
Codecov ReportBase: 63.63% // Head: 63.62% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3538 +/- ##
==========================================
- Coverage 63.63% 63.62% -0.02%
==========================================
Files 309 309
Lines 40873 40900 +27
Branches 5381 5381
==========================================
+ Hits 26011 26021 +10
- Misses 13838 13855 +17
Partials 1024 1024
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Looks like dipy compatibility as well. @skoudoro anything we should know before digging in? I might have a chance this afternoon. |
Nothing particular change. Looking at the error, it seems that a new workflow has a unknown argument type which surprise me. I can look at this only tonight, quite busy today. Thank you for pinging @effigies |
Thanks. I'll let you know if I find anything. If it's just a bug, the easy fix here will be to blacklist the latest release of dipy. Then there shouldn't be a huge rush for you to release. |
Ok, I just looked deeper and you just need to add the type "str" in the function "convert_to_traits_type". This function is handling the keyword "string" but not "str". 😅 It should be an easy and fast fix of this function in nipype. Now, I see why I didn't catch it in DIPY. |
cee2924 to 4622fa9 Compare | @@ -110,7 +110,7 @@ def convert_to_traits_type(dipy_type, is_file=False): | |||
| """Convert DIPY type to Traits type.""" | |||
| dipy_type = dipy_type.lower() | |||
| is_mandatory = bool("optional" not in dipy_type) | |||
| if "variable" in dipy_type and "string" in dipy_type: | |||
| if "variable" in dipy_type and "str" in dipy_type: | |||
There was a problem hiding this comment.
I tested this on 1.5.0 (which was not failing) and 1.6.0 (which was). Both pass.
4622fa9 to a7295cf Compare c3a1909 to f20035c Compare 627d7cf to f6bf0af Compare There was a problem hiding this comment.
LGTM, thanks for the amazing work. Solves the issue I just opened and supersedes my networkx PR.
(I still have one error, but that is fully unrelated to dependency compatibility fixes)
Summary
This PR makes minimal changes to handle deprecations in nibabel 5 and networkx 3.
As seen in #3535, simply switching to sparse arrays breaks the dependency graph matrix representation. However, scipy sparse arrays are just sparse matrices with wrappers on top, so we can just get back to the matrix for now. While they claim they are deprecating the API, much of the API still works with and produces matrices, so I think we have time where we can continue doing what works.
In the long run, it would be nice to understand what is treating these representations differently, but that's beyond the time I have to devote to this. I would rather get things working before embarking on a refactor, especially if it's just a cheap type coercion.
Fixes #3537.
Fixes #3530.
Fixes #3539.
List of changes proposed in this PR (pull-request)
nx.to_scipy_sparse_matrix()tossp.lil_matrix(nx.to_scipy_sparse_array())"str"instead of"string"