Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add take_while and drop_while to Receiver #356

Closed
wants to merge 3 commits into from

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Dec 3, 2024

The take_while() method is just an alias for filter(), with the intention to eliminate the ambiguity and make it more readable.

On the other hand, drop_while() is the negation of take_while() and provided as a convenience and also to improve readability.

These names are widely popular and used in other programming languages as well as the Python itertools module.

Fixes #354.

The `take_while()` method is just an alias for `filter()`, with the
intention to eliminate the ambiguity and make it more readable.

On the other hand, `drop_while()` is the negation of `take_while()` and
provided as a convenience and also to improve readability.

These names are widely popular and used in other programming languages
as well as the Python `itertools` module.

Signed-off-by: Leandro Lucarella <[email protected]>
There is no need to test the methods that are implemented in `Receiver`
itself for every channel or receiver implementation.

We also add a couple of extra trivial tests.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner December 3, 2024 14:19
@llucax llucax self-assigned this Dec 3, 2024
@github-actions github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) labels Dec 3, 2024
@llucax llucax added this to the v1.4.0 milestone Dec 3, 2024
@llucax llucax requested a review from shsms December 3, 2024 14:19
@llucax
Copy link
Contributor Author

llucax commented Dec 3, 2024

I didn't take a lot of time to think if drop_while can also take type guards, but I think it can't, because type guards work only when they are true, if it is false it doesn't mean that the type can't be the type of the guard, it only ensures that when it is true, the type will be narrower.

@shsms
Copy link
Contributor

shsms commented Dec 3, 2024

I didn't take a lot of time to think if drop_while can also take type guards

I haven't looked at the code yet, but I'd imagine drop_while can take a TypeIs, but that probably needs a newer typing-extensions, which we can't do.

@llucax
Copy link
Contributor Author

llucax commented Dec 3, 2024

I think we can bump minor versions, we didn't discuss in depth policies about updates, but I would consider requiring updating dependencies, if they are backwards compatible, not a breaking change. For me a breaking change is something that forces you to update the code, not the tooling.

@llucax
Copy link
Contributor Author

llucax commented Dec 4, 2024

I just looked at TypeIs, and yes, in theory it could allow us to do this, as it is creating a partition of the type, but the problem is Python doesn't have intersection and negation of types as it has type unions (| or Union).

As far as I can see, we would need to declare drop_while() as something like:

    def drop_while(
        self,
        filter_function: Callable[[ReceiverMessageT_co], TypeIs[FilteredMessageT_co]
        ],
        /,
    ) -> Receiver[ReceiverMessageT_co & ^FilteredMessageT_co]:

There seems to be some progress in adding intersection to Python and negation, but I'm not sure how feasible that would be, at least judging from the what the typing docs says about TypeIs:

To specify the behavior of TypeIs, we use the following terminology:

  • I = TypeIs input type
  • R = TypeIs return type
  • A = Type of argument passed to type narrowing function (pre-narrowed)
  • NP = Narrowed type (positive; used when TypeIs returned True)
  • NN = Narrowed type (negative; used when TypeIs returned False)
def narrower(x: I) -> TypeIs[R]: ...

def func1(val: A):
    if narrower(val):
        assert_type(val, NP)
    else:
        assert_type(val, NN)

The return type R must be assignable to I. The type checker should emit an error if this condition is not met.

Formally, type NP should be narrowed to A∧R, the intersection of A and R, and type NN should be narrowed to A∧¬R, the intersection of A and the complement of R. In practice, the theoretic types for strict type guards cannot be expressed precisely in the Python type system. Type checkers should fall back on practical approximations of these types. As a rule of thumb, a type checker should use the same type narrowing logic – and get results that are consistent with – its handling of isinstance(). This guidance allows for changes and improvements if the type system is extended in the future.

Also from this comment from the maintainer of pyright and one of the main contributors to Python typing topics.

@llucax
Copy link
Contributor Author

llucax commented Dec 4, 2024

Oh, dammit, I just realized we misinterpreted what take_while and drop_while do.

  • take_while stops the iteration as soon as one element doesn't fulfill the predicate.
  • drop_while stops using the predicate after an element fulfills the predicate.
>>> print(list(takewhile(lambda x: x < 4, [1, 2, 3, 4, 5, 6])))
[1, 2, 3]
>>> print(list(dropwhile(lambda x: x < 4, [1, 2, 3, 4, 3, 2, 1])))
[4, 3, 2, 1]

So we need new names, or forget about it / add filterfalse().

I will close this PR and move the discussion back to the issue.

@llucax llucax closed this Dec 4, 2024
@llucax llucax added the resolution:wontfix This will not be worked on label Dec 4, 2024
@frequenz-floss frequenz-floss locked as off-topic and limited conversation to collaborators Dec 4, 2024
@llucax llucax modified the milestones: v1.4.0, Dropped Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
part:core Affects the core types (`Sender`, `Receiver`, exceptions, etc.) part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests resolution:wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Receiver.take_while and drop_while
2 participants