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

proposal: chans: new package with chans.Seq #67205

Open
earthboundkid opened this issue May 6, 2024 · 14 comments
Open

proposal: chans: new package with chans.Seq #67205

earthboundkid opened this issue May 6, 2024 · 14 comments
Labels
Milestone

Comments

@earthboundkid
Copy link
Contributor

Proposal Details

#61899 would add iteration related functions to slices, and #61900 does the same for maps. There should also be a package with channel iteration helpers. I propose adding package chans with chans.Seq:

package chans

import "iter"

// Seq returns an iterator that yields the values of ch until it is closed.
func Seq[T any](ch <-chan T) iter.Seq[T] {
    return func(yield func(T) bool) {
        for v := range ch {
            if !yield(v) {
                return
            }
        }
    }
}

An example use would be s := slices.Collect(chans.Seq(ch)) as a convenient way to collect all values from a channel into a slice.

@gopherbot gopherbot added this to the Proposal milestone May 6, 2024
@DeedleFake
Copy link

DeedleFake commented May 6, 2024

I have a package that implements a bunch of iterator-related stuff so that I can play around with it. To supplement the simpler OfChan() function that it provides that does this, I also added a RecvContext() function that can use a context to cancel the iterator, as well as a SendContext() function that does the same in reverse, sending sequence values to a channel until a context is canceled. I'd like to add RecvContext(), under whatever name, for consideration, too.

@earthboundkid
Copy link
Contributor Author

chans.Seq seems definitely necessary. I was thinking we might also want the reverse, which would be like SendContext minus the context part. Putting it together, maybe it should be these four functions to start with:

func Seq[T any](<-chan T) iter.Seq[T]
func SeqContext[T any](context.Context, <-chan T) iter.Seq[T]
func FromSeq[T any](iter.Seq[T]) <-chan T
func FromSeqContext[T any](context.Context, iter.Seq[T]) <-chan T 

There was also talk about having helpers like FanIn and FanOut back when generics were still experimental, but those can be a separate proposal.

@DeedleFake
Copy link

DeedleFake commented May 6, 2024

FromSeq() is wrong. It should be

func FromSeq[T any](iter.Seq[T], chan<- T)

rather than returning a channel.

@earthboundkid
Copy link
Contributor Author

That makes sense. You can reuse an existing channel and you get better type inference.

@earthboundkid
Copy link
Contributor Author

Other names for chans.FromSeq[Context] might be chans.Send or chans.Copy.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals May 6, 2024
@apparentlymart
Copy link

For the two variants that take a context.Context:

I'm accustomed to seeing functions that take a context return an error because an operation being cancelled (or, equivalently, hitting a deadline) is usually modeled as a kind of failure. These functions don't do that, and of course they cannot because the current iterator design only supports infallible iterators.

I can imagine having the caller use ctx.Err() once the iterator stops yielding to see if the iteration might have been terminated early due to cancellation or deadline. It presumably can't know for sure, because the cancellation might have happened between the last call to the iterator and the ctx.Err() call. That edge case probably doesn't matter in practical programs.

s := slices.Collect(chans.SeqContext(ctx, ch))
if ctx.Err() != nil {
    // s may not be complete, then
}

Overall this reminds me of the bufio.Scanner design where users are expected to call Err on the scanner after Scan returns false, to distinguish between a successful full scan vs. early termination due to an error. I remember in one of the earlier iterator proposals some participants worrying that this design was easy to accidentally use incorrectly. That concern would presumably apply to this situation too.

Overall I don't feel super worried about either of these details myself. They certainly don't seem disqualifying, and I raise them largely just for the sake of discussion.

(The non-context versions seem uncontroversial to me; they are pretty obvious implications of the iterators design.)

@earthboundkid
Copy link
Contributor Author

earthboundkid commented May 6, 2024

You could have func SeqContext(context.Context, <-chan T) iter.Seq2[T, error] return the ctx.Err() as the second iterator parameter.

@DeedleFake
Copy link

func FromSeqContext(context.Context, iter.Seq[T], chan<- T) error is possible, too.

@ianlancetaylor
Copy link
Member

Just a note that the main reason I haven't proposed a chans package is exactly the question of how to handle contexts.

@bjorndm
Copy link

bjorndm commented May 8, 2024

How about treating contexts like log/slog did? That would be most consistent and easy to teach.

@AndrewHarrisSPU
Copy link

AndrewHarrisSPU commented May 11, 2024

How about treating contexts like log/slog did? That would be most consistent and easy to teach.

Where slog evolved to omit contexts, I'm not sure the lessons analogize well with iterators; it contrasts with other places the standard library includes contexts.

slog explored a number of approaches with contexts and eventually decided to maximally push the complexity out. Contexts are in the Handler interface, but are never observed in slog's implementations - neither for control flow nor as value stores. The driving questions concerned contexts as value stores, rather than contexts as cancellation in control flows.

Where contexts are used elsewhere in the standard library tends to be domain-specific: net dialing, database/sql queries, http requests, os/exec commands, trace regions and tasks, etc. In each of these cases, there is a pretty useful notion of cancellation that arises in solutions for the domain.

There's undoubtedly a useful notion of cancellation here with chans; also I wonder how often we'd have a more domain-specific iterator, e.g. over the results of a database query, that doesn't already have context cancellation baked in.

@earthboundkid
Copy link
Contributor Author

To get around having a dependency on context, there could be variants that take <-chan struct{} as a cancel channel. It could also be made generic against <-chan struct{} and <-chan time.Time, so it could accept a time.Timer.C.

@earthboundkid
Copy link
Contributor Author

Playground

func Seq[T, Done any](ch <-chan T, done <-chan Done) iter.Seq[T] {
	return func(yield func(T) bool) {
		for {
			select {
			case v, ok := <-ch:
				if !ok {
					return
				}
				if !yield(v) {
					return
				}

			case <-done:
				return
			}
		}
	}
}

func Count() <-chan int { /*...*/ }

func main() {
	for n := range Seq(Count(), time.After(1*time.Microsecond)) {
		fmt.Println(n)
	}
}

Interestingly, this works in a real program but it times out on Playground, presumably because Playground time is an illusion.

@dshearer
Copy link

You could have func SeqContext(context.Context, <-chan T) iter.Seq2[T, error] return the ctx.Err() as the second iterator parameter.

This is my favorite proposal. What are the arguments against it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

8 participants