ledatelescope / bifrost

A stream processing framework for high-throughput applications.
BSD 3-Clause "New" or "Revised" License
66 stars 29 forks source link

New call signature for bf.map #95

Closed benbarsdell closed 7 years ago

benbarsdell commented 7 years ago

It's become apparent to me that the way bf.map currently accepts its function arguments is error-prone and inextensible. Using *args for the index names and **kwargs for the input/output arrays seemed like an elegant solution when I first wrote it, but it means that any use of other arguments (e.g., shape) becomes ambiguous and requires careful handling inside the function. This will get worse in the future, as I have some plans to add additional arguments.

The simplest way to fix it is to change it to something like this:

def map(func_string, data, axis_names=None, shape=None, ...):
    ...
bf.map("c(i,j) = a(i) * b(j)", data={'c': c, 'a': a, 'b': b}, axis_names=['i','j'], ...)

which is somewhat uglier than the current formulation, but avoids any ambiguity.

I wanted to canvass opinions on the new signature. Is the above new version OK? Any suggestions?

MilesCranmer commented 7 years ago

Looks great to me!

telegraphic commented 7 years ago

Agreed, the dictionary is a relatively clean approach. The only other approach I could imagine is a bfMap 'object' that you instantiate, e.g.:

bmap = BfMap()
bmap.func_string = "c(i,j) = a(i) * b(j)"
bmap.data = ...
bmap.axis_names = ...

bf.map(bmap)

but honestly I prefer the one liner.

benbarsdell commented 7 years ago

Thanks guys, I've pushed the changes. You'll need to update the calls in your pipelines if you have any.