AlexandreDecan / portion

portion, a Python library providing data structure and operations for intervals.
GNU Lesser General Public License v3.0
476 stars 34 forks source link

Feature Request: `how` callable to receive intervals #97

Closed egemenimre closed 1 month ago

egemenimre commented 1 month ago

TL;DR: can you modify how signature in IntervalDict.combine() method to receive the active interval and to look like this:

how(v1, v2, i=i)

The optionals should make it backwards compatible, and no existing code would be broken.

Rationale: I'm writing a code where I have an IntervalDict, where each interval has an associated interpolator. When two IntervalDicts are combined, then I'd like to do a piecewise multiplication with discrete samples and then generate a new interpolator.

Concrete example: Imagine the light spectrum from the Sun. When combined with a "filter", the passing light has a single colour. If the interpolator of the solar spectrum is intersected with the filter (1 in the bandpass region, 0 otherwise), then the resulting object would have zero in the entire domain, with the exception of the bandpass region. If the filter has a more complex shape, then two interpolators will have to interact, where I will have to do all the resampling and interpolator generation within the how callable.

Currently, the callable does not receive the i interval, therefore it does not know for which part of the domain it is called.

AlexandreDecan commented 1 month ago

Hello,

I'm not sure I fully got the use-case, but I think I see the point :-) I'm not against the change, but I don't see how changing the signature of how to how(v1, v2, i=i) makes it backward compatible.

What about adding an extra parameter on the combine method, e.g., pass_interval=False that, if set to True, makes combine call how with the current interval as first parameter? E.g., instead of how(v1, v2) for all intersecting i, if pass_interval = True, we call how(i, v1, v2).

egemenimre commented 1 month ago

Thanks for the reply.

Of course, adding the third item i would break all the legacy code. My bad, posting too late at night is rarely a good idea. :)

Then, as you say, the only easy option could be to ask the user with a flag and let combine handle the two options - with or without the interval. I just suggest that i could be the last item in the call rather than the first, simply to make sure the two and three input versions look similar in signature.

Another funky (as opposed to easy, and maybe too funky for its own good) idea could be to inspect the how for the number of parameters. This would save the user from specifying the flag each and every time, but introduces a danger that the user may shuffle the signature and we are making an assumption regarding the order:

import inspect

def combine(self, other, how, *, missing=...):
       ...

        for i1, v1 in d1.items():
            for i2, v2 in d2.items():

                if i1.overlaps(i2):
                    i = i1 & i2

                    # this should be 2 or 3, depending on how the user defines how
                    nr_of_params = len(inspect.signature(how).parameters)

                    if nr_of_params == 2:
                           v = how(v1, v2)
                    else:
                           # this assumes nr_of_params == 3
                            v = how(v1, v2, i)

                    new_items.append((i, v))
egemenimre commented 1 month ago

Also, how can receive a (*args, **kwargs), passed on from combine, to enable an even more versatile how. This way I can pass my own parameters that I use within the how functionality.

AlexandreDecan commented 1 month ago

Hi,

Having the interval as third item is indeed better than as first parameter.

I tend to dislike the funky solution since I'm afraid this would make the code more fragile and I'm not sure inspect works correctly this way for all cases (e.g., functions accepting a variable number of parameters, or functions defining default values for some parameters).

About the second message, I'm against the idea. The same behaviour can be obtained by using partial for the how function before passing it to combine so I don't see the benefit of adding this "complexity" to combine :-)

egemenimre commented 1 month ago

I fully understand the opposition to the funky solution. :)

Can you elaborate a bit on partial? Maybe a simple example?

The usecase is where I need to pass on another variable (say, a k that gets multiplied with the two functions). They need to go all the way inside how, so that I can do something like:

def how_funct(fx, fy):

   # don't know where k comes from now
   combined = fx * fy * k
   return  combined

I'm open to ideas as to how I can achieve this.

AlexandreDecan commented 1 month ago

Sure. If I understand correctly, you want to add *args and **kwargs to the combine method, so that they are passed to the how function when it is called. So, to some extent, every call to how(x, y) will be replaced by how(x, y, *args, **kwargs), right? Let's say you have a f function taking 3 parameters (this could be your how_funct above), namely x, y, z where z should be known in advance. This means you want to call d1.combine(d2, f, z=3) for example. With the current implementation, this call can be replaced by d1.combine(d2, functools.partial(f, z=3)).

egemenimre commented 1 month ago

Ah, clever. Didn't know about partials before. That would indeed do the trick.

Maybe you can add a recipe somewhere, as it really enhances the combine functionality.

And as for the addition of the active interval to how, when can I expect it? do you have a minor release lined up? :)

AlexandreDecan commented 1 month ago

partial is quite common, the same trick can also be achieved using a lambda function. I do not think this deserves some attention in the documentation, though ;-)

Considering the pass_interval parameter, I have no ETA yet. I need to work on it (won't take much time), write some tests, some documentation and release a new minor version. I'll see when I have some free time for it ;-)

AlexandreDecan commented 1 month ago

Hello,

The 2.6.0 release should be soon available on PyPI. I hope it does the job, I haven't written much tests... Even if the implementation is quite straightforward, I should add more :-)