Signal filter_map problem

[deiwo]

Hello, I used the filter_map function on cloned signal and the output it provides is encapsulated in Option, instead of filter disposing of the Option as is in normal iterator filter_map function.

I managed to obtain desired result with this change to Signal trait implementation for FilterMap:

impl<A, B, C> Signal for FilterMap<A, B>
    where A: Signal,
          B: FnMut(A::Item) -> Option<C> {
    type Item = C;

    // TODO should this use #[inline] ?
    fn poll_change(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        let FilterMapProj { mut signal, callback, first } = self.project();

        loop {
            return match signal.as_mut().poll_change(cx) {
                Poll::Ready(Some(value)) => match callback(value) {
                    Some(value) => {
                        *first = false;
                        Poll::Ready(Some(value))
                    },

                    None => {
                        if *first {
                            *first = false;
                            Poll::Ready(None)

                        } else {
                            continue;
                        }
                    },
                },
                Poll::Ready(None) => Poll::Ready(None),
                Poll::Pending => Poll::Pending,
            }
        }
    }
}

I don't know if the current behavior is desired or not, maybe I am just not understanding this function correctly.

[Pauan]

Hello, I used the filter_map function on cloned signal and the output it provides is encapsulated in Option, instead of filter disposing of the Option as is in normal iterator filter_map function.

Yes, that is expected. A Signal is similar to a mutable variable, it must always have a value. So if the filter_map returns None then what value should the Signal have? The only sensible value it can have is None.

The purpose of filter_map is to avoid updates: if the closure returns None then the downstream consumers know that the Signal isn't updating. This is more efficient, since it won't trigger any changes in the downstream consumers.

[deiwo]

Yes I agree, but the behavior that I am experiencing is that the result of filter_map is not passed as the value of signal. Which means that the value passed in Signal is Option<A> entering into filter_map and Option<Option<B>> exiting. So one layer of encapsulation is added. Shouldn't it be that the Option returned from filter_map is directly passed to next Signal?

[Pauan]

The Option returned from filter_map is directly passed to the next Signal:

some_signal
    .filter_map(|_| Some(5))
    .map(|x| {
        // x == Some(5)
    })

If the input is Option<A> and you return it, then it will also be passed through:

some_signal
    .filter_map(|value| value)
    .map(|x| {
        // x: Option<A>
    })

If you want to do some sort of transformation on it, then you should use the Option methods like map/and_then:

some_signal.filter_map(|value| {
    value.and_then(|value| {
        Some(value + 5)
    })
})

Or you can use ?:

some_signal.filter_map(|value| {
    let value = value?;
    Some(value + 5)
})

[deiwo]

Ok, but why pass the Option::None as value to the next Signal and have the user manually process the situation when the value did not change. Rather than, in FilterMap directly pass the Poll::Ready(None) to the next signal which would mean that the possible next closures don't even get called? It would also make the processing easier by passing the value directly to the next closure rather than encapsulating it in Option.

For example here, the map closure will be called only if the filter_map returned Some(...), because if it returned None then no change in the signal was made:

some_signal
    .filter_map(|value| if value > 5 {Some(value - 5)} else {None})
    .map(|value| value*10)

But currently the code would need to look something like you wrote earlier:

some_signal
    .filter_map(|value| if value > 5 {Some(value - 5)} else {None})
    .map(|value| {
        let val = value?;
        val*10})

Which I think is unnecessary, or is there some other use case where this might be helpful?

[Pauan]

Ok, but why pass the Option::None as value to the next Signal and have the user manually process the situation when the value did not change.

Like I said, the reason is to avoid unnecessary updates. If filter_map returns None then the downstream Signals won't be updated, so this is better for performance.

There are several methods that don't change the value, they exist only to prevent unnecessary updates, such as dedupe, or throttle. Avoiding unnecessary updates is useful, it improves performance.

It would also make the processing easier by passing the value directly to the next closure rather than encapsulating it in Option.

As I said before, that is not possible. A Signal must always have a value. If the filter_map returns None, then what value should be returned for the Signal?

[deiwo]

I think I understand now, but I needed to go over the code once again. As I understand it, the problem comes from one situation and that is on the beginning. If for the very first value the filter_map returns None there would be no value for the lower Signals to have. That is why there has to be Option returned from the filter_map Signal and the default value of None otherwise the Signal will always be Some(...). (As is written in the documentation, but I did not understand what it meant.)

So if the filter_map returns None then what value should the Signal have? The only sensible value it can have is None.

This sentence confused me and now I understand :) , because this is on the very beginning when no values were returned as Some(...) from filter_map.

Thank you for your patience with me.

[Pauan]

If for the very first value the filter_map returns None there would be no value for the lower Signals to have.

That's correct. That's also why from_stream must return an Option, because if the Stream is empty then it must return None for the first value.

Perhaps it would make sense to rename filter_map, so it's more clear what it does.

[deiwo]

I think the name is fine and it makes sense, now that I understand what is the point. But it may be confusing, for others that expect it to behave like standard filter_map (as I did), but they get Option as the result. (Which was confusing and I assumed that it is the exact same Option that the closure from filter_map returns.)

[Pauan]

Which was confusing and I assumed that it is the exact same Option that the closure from filter_map returns.

Your assumption is correct, the Option that is returned from filter_map is passed onto the downstream Signals.

Perhaps what you're misunderstanding is how the Signal trait works. The poll_change method returns a Poll<Option<Self::Item>>. The Poll indicates whether the value has changed or not, and the Option indicates whether the Signal has ended or not.

In the case of filter_map, it returns a Poll<Option<Option<C>>>. The first Option indicates whether the Signal has ended or not, and the second Option is the actual value returned by the Signal.

This is similar to how Iterator.next returns Option<Self::Item>, with None meaning that the Iterator has ended. An Iterator<Item = Option<C>> would return Option<Option<C>>, because the first Option indicates whether the Iterator has ended or not, and the second Option is the actual value.

You're not supposed to call poll_change yourself, instead you're supposed to use something like for_each, which will unwrap the first Option for you:

some_signal
    .filter_map(|_| Some(5))
    .for_each(|value| {
        // value == Some(5)
        async {}
    })

Of course the other combinators also unwrap the first Option, since it's an internal implementation detail of the Signal system, the first Option is never exposed to the user. Similarly, the Poll is also never exposed to the user, it's an implementation detail.