nipype / pydra

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

FIX: Allow staticmethod be passed to callable metadata in ouput spec #632

Closed ghisvail closed 1 year ago

ghisvail commented 1 year ago

Types of changes

Summary

This PR provides a fix allowing the callable field metadata be defined as a staticmethod to the output spec class.

Before this fix, the following would fail:


@attrs.define(kw_only=True)
class MyOutSpec(pydra.specs.ShellOutSpec):

    @staticmethod
    def get_foo(field):
        return "foo.bar"

    foo: pydra.specs.File = attrs.field(
        metadata={
            help="some output help",
            callable=get_foo,
        }
    )

The reason for that is that the actual callable is wrapped inside the staticmethod descriptor and does not expose the Callable interface, which makes runtime reflection with inspect fail with a TypeError. The solution is just to recover the wrapped method inside the descriptor.

Checklist

satra commented 1 year ago

@ghisvail - can you add your example as a test?

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.05 :warning:

Comparison is base (374cdec) 81.76% compared to head (73f830e) 81.72%.

:exclamation: Current head 73f830e differs from pull request most recent head 46a6e9a. Consider uploading reports for the commit 46a6e9a to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #632 +/- ## ========================================== - Coverage 81.76% 81.72% -0.05% ========================================== Files 20 20 Lines 4392 4393 +1 Branches 1264 0 -1264 ========================================== - Hits 3591 3590 -1 - Misses 797 803 +6 + Partials 4 0 -4 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `81.72% <100.00%> (-0.05%)` | :arrow_down: | 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=None#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/nipype/pydra/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [pydra/engine/helpers\_file.py](https://codecov.io/gh/nipype/pydra/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cHlkcmEvZW5naW5lL2hlbHBlcnNfZmlsZS5weQ==) | `86.10% <ø> (ø)` | | | [pydra/utils/profiler.py](https://codecov.io/gh/nipype/pydra/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cHlkcmEvdXRpbHMvcHJvZmlsZXIucHk=) | `45.80% <ø> (ø)` | | | [pydra/engine/specs.py](https://codecov.io/gh/nipype/pydra/pull/632?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cHlkcmEvZW5naW5lL3NwZWNzLnB5) | `94.78% <100.00%> (+0.01%)` | :arrow_up: | ... and [4 files with indirect coverage changes](https://codecov.io/gh/nipype/pydra/pull/632/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

djarecka commented 1 year ago

thanks! lgtm