[fix] fix for output that are numpy.array (closes #270, #339) #340
Conversation
…xes, a test added)
Codecov Report
@@ Coverage Diff @@
## master #340 +/- ##
=======================================
Coverage 85.80% 85.80%
=======================================
Files 20 20
Lines 3713 3713
Branches 1000 1000
=======================================
Hits 3186 3186
Misses 337 337
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -156,7 +156,7 @@ def copyfile_workflow(wf_path, result): | |||
| for field in attr_fields(result.output): | |||
| value = getattr(result.output, field.name) | |||
| new_value = _copyfile_single_value(wf_path=wf_path, value=value) | |||
| if new_value != value: | |||
| if not (new_value is value or new_value == value): | |||
There was a problem hiding this comment.
I am afraid that this won't work for numpy arrays new_value and value that are different from each other.
It will result in a boolean array (same size results in elementwise comparison) or raises an exception in the future (elementwise comparison is deprecated for arrays of different sizes).
There was a problem hiding this comment.
this is why I added new_value is value part for numpy arrays.
Do you have an example that doesn't work with it? Could you please post it. I believe the one you send in #270 should work (added very similar test)
There was a problem hiding this comment.
Sorry, I was wrong I guess. Because of _copyfile_single_value a case like value = np.array([1,2,3]) and new_value = np.array([4,5,6]) will never happen, right?
There was a problem hiding this comment.
@daniel-ge - sorry, but I've missed the main problem that the numpy arrays should not be here at all. I will review the code more carefully and post a new PR
|
open a new pr: #344 |
Types of changes
Summary
yet another place that should be fixed for numpy.array as an output (test added)
Checklist
(we are using
black: you canpip install pre-commit,run
pre-commit installin thepydradirectoryand
blackwill be run automatically with each commit)Acknowledgment