Add only function

[nickrobinson251]

• Rebases @andyferris's PR Add only function #25078
• Updates it where necessary
• Adds a few more tests

[nickrobinson251]

failed tests on Windows look unrelated

Edit: yep, now windows passes and Mac has an unrelated failure :)

[andyferris]

Thanks, @nickrobinson251!

[c42f]

I'm late to the bikeshed here, but what about renaming to takeonly? We have the somewhat-precedent of take(iter, n) and I feel this slightly longer name might be helpful for an exported function which is somewhat obscure.

[nickrobinson251]

i'm late to the bikeshed here, but what about renaming to takeonly? We have the somewhat-precedent of take(iter, n)

The idea of a takeonly(itr, n) has come up, but it just seems like a different function. And we'd want the name for this other function. (#25078 (comment))

only here is like first or last with an added assertion. It returns a single element, not an iterator. only as a name also has precendence in other languages,for what that's worth.

(For example, i could imagine a takeonly which iterates the first n elemens and assert that is all the elements. takeonly would be to take whatonly is to first. Although personally I've never needed such a function. Whereas I regularly think I've narrowed some collection down to one element.)

In general, the last round of naming discussion (in #25078) I thnk came back round to only. Keen not to see this stall too much on naming, if there's broad support for the functionality :)

[nickrobinson251]

Since this returns an element of a collection, not an iterator, I could move it to a different file, if that's preferable?

I followed the previous PR in putting it in Iterators.jl, also because it works for any collection. But e.g. I could put it in the same file as first and last instead (they are in abstractarray.jl even though they also work on any collection)?

Let me know :)

(Edit: I think being with first and last would be a good move, so i'll probably do this.)

[oxinabox]

only matchs F#,
C# calls this Single IIRC, idk about other languages, anyone?

I much prefer the name only to takeonly because it is short, like first and last.
and I think a great many uses of first can be replaced with only.
Where as takeonly is longer and thus feels more obscure.
But really I think this should be used about as often as first (if not more),
and more than last.

I also like the idea of holding takeonly for the version that is takeonly(xs, n) returning an iterator of length n)

[c42f]

It returns a single element, not an iterator.

Yes, I think this is a convincing argument that take should not be in the name 👍

[andyferris]

To me the only reasonable candidates are only and single. (Interesting that F# went with only, differently to C#). I honestly don’t think we’re going to find conclusive arguments either way though - so I’d support simply merging this.

[c42f]

Since this returns an element of a collection, not an iterator, I could move it to a different file, if that's preferable?

I think what you have is fine and it is first and last which are misplaced (the documentation defines first in terms of iteration so in my view it should be in iterators.jl).

[c42f]

Overall this looks very neat to me. Given the precedent for naming and the other arguments offered for only I think we should just go with it.

The freebsd build error is unrelated (a system problem on the build slave?).

[nickrobinson251]

I sort of feel the code makes more sense in base/abstractarray.jl, to be with first and last, and because it doesn't return an iterator.

But I didn't really know where to move the tests to... Except scattering them across files. So I've left it for now.

In looking at tests for first/last, I did notice we weren't testing only on Pairs, so I added that test.

[nickrobinson251]

Did this PR get discussed in the triage call this week?

Please let me know if anything needs doing here in order for it to get merged.

Thanks for the reviews so far :)

[JeffBezanson]

Triage approves; merge when tests pass.

[nickrobinson251]

Tests had a silly typo, sorry. Now it looks like they didn't all run on latest commit?

I only seepackage_wind64, which failed with

make: *** No rule to make target 'win-extras'.  Stop.
program finished with exit code 2

Is someone able to re-trigger tests?

[c42f]

I've retriggered those two tests for freebsd and win64 (though hard to see how the failures could be at all related).

[c42f]

I merged this - the single win64 CI error was a spurious problem in the FileWatching tests.

It seems I don't understand how github's squash-and-merge functionality decides on the author, so Nick is listed redundantly in the Co-Authored-By (sorry!)