Skip to content

replace extra method with string parameter #24

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

replace extra method with string parameter #24

wants to merge 2 commits into from

Conversation

bakkot
Copy link
Collaborator

@bakkot bakkot commented Jul 22, 2025

I really don't like having two almost identical methods which differ only in their handling of an edge case. This PR removes sliding and adds a string parameter to windows which controls the behavior in the case of an undersized iterator, with possible values "discard" (the default) or "truncate" (the previous behavior for sliding). Feel free to bikeshed the values if you have strong opinions. I left out "throw" but I guess it's also a valid thing to want (edit: #25).

I've chosen to go with a string argument instead of a boolean so that we can later turn this into an options bag if we really feel the need. Boolean would also be fine but makes it harder to do that change (because we said we'd allow coercion to boolean).

Fixes #23 (by causing it not to be a problem).

@bakkot
Copy link
Collaborator Author

bakkot commented Jul 29, 2025

In plenary today @waldemarhorwat suggested doing "discard" and "short", which I'm also happy with.

@michaelficarra
Copy link
Member

I think the parameter should use a consistent part of speech, preferably verbs. Past participles "discarded"/"omitted" and "undersized"/"truncated" would also be fine.

@bakkot
Copy link
Collaborator Author

bakkot commented Jul 29, 2025

"Discard", noun, "one that is cast off or rejected".

@michaelficarra
Copy link
Member

😛 That's not the form being used here and you know it.

@js-choi
Copy link

js-choi commented Jul 29, 2025

What about “discard” and “shorten”?

@michaelficarra
Copy link
Member

That seems fine. It's pretty much synonymous with truncate to me 🤷‍♂️.

@jridgewell
Copy link
Member

I would throw in partial, since that can't be confused with truncating the input array. Maybe full/partial?

@bakkot
Copy link
Collaborator Author

bakkot commented Jul 29, 2025

"full" is confusing to me because you're getting fewer outputs in that case. I'd maybe accept "only-full"? But that's kind of verbose.

@bakkot
Copy link
Collaborator Author

bakkot commented Aug 11, 2025

To address part of speech concerns, "discard" and "output-short", maybe?

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

Successfully merging this pull request may close these issues.

Name of .sliding
4 participants