nipype / pydra

Pydra Dataflow Engine
https://nipype.github.io/pydra/
Other
120 stars 59 forks source link

Hash change guards #698

Closed tclose closed 6 months ago

tclose commented 1 year ago

Types of changes

Summary

Addresses #681 issue

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Attention: Patch coverage is 97.05882% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 83.93%. Comparing base (0e66136) to head (ff281aa).

Files Patch % Lines
pydra/engine/submitter.py 88.23% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #698 +/- ## ========================================== + Coverage 83.65% 83.93% +0.27% ========================================== Files 24 24 Lines 4986 5029 +43 Branches 1416 1429 +13 ========================================== + Hits 4171 4221 +50 + Misses 809 802 -7 Partials 6 6 ``` | [Flag](https://app.codecov.io/gh/nipype/pydra/pull/698/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/nipype/pydra/pull/698/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype) | `83.93% <97.05%> (+0.27%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipype#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tclose commented 1 year ago

I believe this is good for review now. The new SLURM test keeps on getting stuck somewhere but I assume that it isn't related

tclose commented 1 year ago

NB: I'm not sure why the coverage is down. It seems to be an issue with the dask submitter code

djarecka commented 1 year ago

indeed looks like the coverage has dropped, what is weird, since for some time we didn't test Dask at all...

tclose commented 1 year ago

indeed looks like the coverage has dropped, what is weird, since for some time we didn't test Dask at all...

I don't trust codecov in this instance (and others). The dask test is passing so it couldn't be missing the Dask submitter, which is where the bulk of the coverage drop comes from (otherwise it would be a coverage increase I believe)

tclose commented 7 months ago

@djarecka I believe this is ready to merge now

djarecka commented 6 months ago

I'm good with merging, thank you @tclose !