Add support for arbitrary mappings in a FunctionTask #565
Conversation
|
For a concrete example, it would simplify this piece of terrible code very much: https://github.com/aramis-lab/pydra-bids/blob/main/pydra/tasks/bids/utils/read_bids.py |
|
A safer alternative could also be to only return if the mapping returned by the inner function contains all keys declared in |
|
a side note: for dynamic tasks as being targeted here, the cache checksum/location will only depend on inputs (e.g. the query string/template), not the underlying database. hence either the source directory would need to be provided and cached as a separate input(a potentially expensive operation), or another default input should be associated that uses a random number generator as a default. the current forcing option of |
|
Good point. How does caching work today with nipype for things like |
caching is local to a workflow and nipype1 does not have the concept of reusing the same task in multiple places or even across workflows. this is only available in pydra. in nipype1, simply setting the rerun flag would force that interface to rerun, but because there is no shared rerun capabilities, this particular issue does not come into play. so dynamic interfaces are simply set to rerun by default in nipype1. |
Codecov ReportBase: 81.23% // Head: 38.26% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #565 +/- ##
===========================================
- Coverage 81.23% 38.26% -42.97%
===========================================
Files 20 20
Lines 4391 4393 +2
Branches 1262 1202 -60
===========================================
- Hits 3567 1681 -1886
- Misses 820 2416 +1596
- Partials 4 296 +292
Flags with carried forward coverage won't be shown. Click here to find out more.
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. |
|
@ghisvail - can't access the link with the example anymore. perhaps you can also add some test, e.g. |
|
Yes, I am planning to add some unit tests covering this particular usage. I'll ping you once done :+1 |
ghisvail
force-pushed
the
feat/run-task-dict
branch
2 times, most recently
from
2b5d79a
to
4d765c8
Compare
ghisvail
force-pushed
the
feat/run-task-dict
branch
from
0f8869b
to
b89f9a2
Compare
|
I added a test case which covers present, absent and undefined mappings from returned A natural extension to this PR would be to handle functions annotated with a TypedDict return value. |
I did some refactoring of the code in the meantime. This is what it looks like today: https://github.com/aramis-lab/pydra-bids/blob/main/pydra/tasks/bids/utils.py Granted it's likely a terrible way of making dynamic tasks for reasons I have not thought of, but I sense there could be a nice pattern emerging from this. |
|
@djarecka do you think we could get this one in for the release too? |
|
I was also thinking that extending this feature to class Point(TypedDict):
x: int
y: int
@task
def random_point() -> Point:
return Point(x=random.randrange(16), y=random.randrange(16))And have the resulting task automatically expose Maybe? |
|
if you think that you could use it, then sure |
|
Going forward, I think it would be good to deprecate |
We might not need to deprecate it, since What do you think of this plan? |
|
Sounds good to me. |
|
Ok, then I'll find some time to start hacking on it 👍 |
|
@ghisvail - you want to still work in this PR, or you want me to merge this one and open a new one? |
|
@djarecka please go ahead with merging. I'll open a follow-up PR for |
Types of changes
Summary
This PR adds support for returning total or partial mappings of the output spec by the wrapped function, as an alternative to going through a conversion from the returned tuple to a dictionary via
output_names. Unlike the named tuple case where the shape of the data is known a priori (I have got exactly 2 output variables which I want named tofooandbar), this PR adds support for functions which may return more, less or exactly these data as an arbitrary mapping.This could be particularly useful for dynamic task construction use cases, like BIDS readers, parsers and writers.
Checklist