[PATCH] seq.el: add seq-last for symmetry with seq-first

[PATCH] seq.el: add seq-last for symmetry with seq-first

[Matúš Goljer]

[-- Attachment #1: Type: text/plain, Size: 202 bytes --]

Sometimes I want to get the last element of sequence and
(seq-elt seq (1- (seq-length seq)) adds too much noise.

This patch adds a simple wrapper around that.
-- 
Best regards,
  Matúš Goljer

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] seq.el: add seq-last for symmetry with seq-first --]
[-- Type: text/x-patch, Size: 823 bytes --]

From f3ef1a282d0aa1f4c2a21c946845516d0caf8c31 Mon Sep 17 00:00:00 2001
From: Matus Goljer <matus.goljer@gmail.com>
Date: Tue, 14 Mar 2023 13:46:10 +0100
Subject: [PATCH] seq.el: add seq-last for symmetry with seq-first

---
 lisp/emacs-lisp/seq.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 6917c541f2e..2571b558c95 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -124,6 +124,10 @@ name to be bound to the rest of SEQUENCE."
   "Return the first element of SEQUENCE."
   (seq-elt sequence 0))
 
+(defun seq-last (sequence)
+  "Return the last element of SEQUENCE."
+  (seq-elt sequence (1- (seq-length sequence))))
+
 (defun seq-rest (sequence)
   "Return SEQUENCE with its first element removed."
   (seq-drop sequence 1))
-- 
2.34.1

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Philip Kaludercic]

Matúš Goljer <matus.goljer@gmail.com> writes:

> Sometimes I want to get the last element of sequence and
> (seq-elt seq (1- (seq-length seq)) adds too much noise.

Wouldn't it make sense to add a specialised implementation for lists, to
avoid recusing the list twice.

A thing I notice is that seq-first is not consistent on the way it
behaves if the sequence is empty.

        (seq-first '()) ;=> nil

while

        (seq-first [])

raises an error.  seq-last would have the same issue for vectors, except
that it would attempt to index the position -1, which might be
confusing?

> This patch adds a simple wrapper around that.
> -- 
> Best regards,
>   Matúš Goljer
>
> From f3ef1a282d0aa1f4c2a21c946845516d0caf8c31 Mon Sep 17 00:00:00 2001
> From: Matus Goljer <matus.goljer@gmail.com>
> Date: Tue, 14 Mar 2023 13:46:10 +0100
> Subject: [PATCH] seq.el: add seq-last for symmetry with seq-first
>
> ---
>  lisp/emacs-lisp/seq.el | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index 6917c541f2e..2571b558c95 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -124,6 +124,10 @@ name to be bound to the rest of SEQUENCE."
>    "Return the first element of SEQUENCE."
>    (seq-elt sequence 0))
>  
> +(defun seq-last (sequence)
> +  "Return the last element of SEQUENCE."
> +  (seq-elt sequence (1- (seq-length sequence))))
> +
>  (defun seq-rest (sequence)
>    "Return SEQUENCE with its first element removed."
>    (seq-drop sequence 1))

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Matúš Goljer]

> Wouldn't it make sense to add a specialised implementation for lists, to
> avoid recusing the list twice.

I can turn it into defgeneric with default implementation same as what I
provided and special instance for a list, that makes sense.  Although I
wonder what the performance impact is of the dispatch vs iterating a
list /shrug.

> A thing I notice is that seq-first is not consistent on the way it
> behaves if the sequence is empty.
>
>         (seq-first '()) ;=> nil
>
> while
>
>         (seq-first [])
>
> raises an error.  seq-last would have the same issue for vectors, except
> that it would attempt to index the position -1, which might be
> confusing?

I think for lists it should behave as `nth` or `elt`, so it gives nil.
I agree that the error with -1 for vector might be confusing.  Should we
instead raise our own (seq) error for empty vector with seq-first and
seq-last?  Not sure what would be the best way.

I should also add some tests, I've noticed there was a test file for
seq.  Should I resubmit this patch with the feedback incorporated to
bug-gnu-emacs@gnu.org instead?  I'm not sure if emacs-devel is the
place, sorry T_T.

-- 
Best regards,
  Matúš Goljer

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Philip Kaludercic]

Matúš Goljer <matus.goljer@gmail.com> writes:

>> Wouldn't it make sense to add a specialised implementation for lists, to
>> avoid recusing the list twice.
>
> I can turn it into defgeneric with default implementation same as what I
> provided and special instance for a list, that makes sense.  Although I
> wonder what the performance impact is of the dispatch vs iterating a
> list /shrug.

That would be worth investigating, but considering that your proposed
implementation would already have two dispatches (seq-elt and
seq-length), I don't think this will be much worse, especially when the
sequence becomes longer.

>> A thing I notice is that seq-first is not consistent on the way it
>> behaves if the sequence is empty.
>>
>>         (seq-first '()) ;=> nil
>>
>> while
>>
>>         (seq-first [])
>>
>> raises an error.  seq-last would have the same issue for vectors, except
>> that it would attempt to index the position -1, which might be
>> confusing?
>
> I think for lists it should behave as `nth` or `elt`, so it gives nil.

But why?  Wouldn't that be a leaky abstraction, since the behaviour
doesn't consistently abstract over the concrete sequence types?  If code
doesn't want to worry about what sequence is being used, then it has to
manually check the return value or if a signal was raised, depending on
the type of the argument to seq-last (which is to ignore the issue that
we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
the same also being the case for seq-last).

> I agree that the error with -1 for vector might be confusing.  Should we
> instead raise our own (seq) error for empty vector with seq-first and
> seq-last?  Not sure what would be the best way.

I believe this would always be better, even for seq-first, though it
might be that too late for that?

> I should also add some tests, I've noticed there was a test file for
> seq.  Should I resubmit this patch with the feedback incorporated to
> bug-gnu-emacs@gnu.org instead?  I'm not sure if emacs-devel is the
> place, sorry T_T.

IIRC patches should be sent to bug-gnu-emacs@gnu.org, but emacs-devel is
also acceptable if there is a need for discussion.  I think the points I
raised above would be worth a general discussion on how seq is to be
used, which is not the matter of a single bug report.

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Augusto Stoffel]

Maybe it's more useful to allow negative arguments in seq-elt?  Saying
(seq-elt seq -1) isn't much more effort than (seq-last seq).

On Tue, 14 Mar 2023 at 16:14, Philip Kaludercic wrote:

>> I think for lists it should behave as `nth` or `elt`, so it gives nil.
>
> But why?  Wouldn't that be a leaky abstraction, since the behaviour
> doesn't consistently abstract over the concrete sequence types?  If code
> doesn't want to worry about what sequence is being used, then it has to
> manually check the return value or if a signal was raised, depending on
> the type of the argument to seq-last (which is to ignore the issue that
> we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
> the same also being the case for seq-last).

It would be good to look systematically at what errors seq.el can
signal.  But it also seems that in practice the main value of seq.el is
to provide a bunch of handy functions rather than allowing you to work
with a sequence whose type you don't know.

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Matúš Goljer]

> Maybe it's more useful to allow negative arguments in seq-elt?  Saying
> (seq-elt seq -1) isn't much more effort than (seq-last seq).

I'm personally a bit ambiguous about negative indices.  It's nice for
those one liners when you need them, but they are quite confusing
especially if you switch between languages and they all implement them a
bit differently.

Sometimes having simple semantics of "last item" is better for
understanding the code 6 months down the line or when just skimming
through.

But of course we can have both.  Emacs itself has `car` but `nth 0` also
works.

>>> I think for lists it should behave as `nth` or `elt`, so it gives nil.
>>
>> But why?  Wouldn't that be a leaky abstraction, since the behaviour
>> doesn't consistently abstract over the concrete sequence types?  If code
>> doesn't want to worry about what sequence is being used, then it has to
>> manually check the return value or if a signal was raised, depending on
>> the type of the argument to seq-last (which is to ignore the issue that
>> we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
>> the same also being the case for seq-last).
>
> It would be good to look systematically at what errors seq.el can
> signal.  But it also seems that in practice the main value of seq.el is
> to provide a bunch of handy functions rather than allowing you to work
> with a sequence whose type you don't know.

Yea, this has been my experience as well.  Usually I know what the
sequence is, but seq provides a nice interface to not have to *remember*
how to do what I need.  Just seq-do it and it will work somehow.

-- 
Best regards,
  Matúš Goljer

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Philip Kaludercic]

Augusto Stoffel <arstoffel@gmail.com> writes:

> Maybe it's more useful to allow negative arguments in seq-elt?  Saying
> (seq-elt seq -1) isn't much more effort than (seq-last seq).
>
> On Tue, 14 Mar 2023 at 16:14, Philip Kaludercic wrote:
>
>>> I think for lists it should behave as `nth` or `elt`, so it gives nil.
>>
>> But why?  Wouldn't that be a leaky abstraction, since the behaviour
>> doesn't consistently abstract over the concrete sequence types?  If code
>> doesn't want to worry about what sequence is being used, then it has to
>> manually check the return value or if a signal was raised, depending on
>> the type of the argument to seq-last (which is to ignore the issue that
>> we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
>> the same also being the case for seq-last).
>
> It would be good to look systematically at what errors seq.el can
> signal.  But it also seems that in practice the main value of seq.el is
> to provide a bunch of handy functions rather than allowing you to work
> with a sequence whose type you don't know.

I don't think the two are necessarily different issues.  Using seq might
incur a dispatch overhead, but you get the advantage that your code is
less bound to a specific data structure and is therefore easier to
adjust later on because you rely on the abstract behaviour instead of a
concrete structure.

Matúš Goljer <matus.goljer@gmail.com> writes:

>> Maybe it's more useful to allow negative arguments in seq-elt?  Saying
>> (seq-elt seq -1) isn't much more effort than (seq-last seq).
>
> I'm personally a bit ambiguous about negative indices.  It's nice for
> those one liners when you need them, but they are quite confusing
> especially if you switch between languages and they all implement them a
> bit differently.

The only language I am really familiar with is python, and what that
effectively does is (mod i (length n)), what do other languages do?

> Sometimes having simple semantics of "last item" is better for
> understanding the code 6 months down the line or when just skimming
> through.
>
> But of course we can have both.  Emacs itself has `car` but `nth 0` also
> works.

True.

>>>> I think for lists it should behave as `nth` or `elt`, so it gives nil.
>>>
>>> But why?  Wouldn't that be a leaky abstraction, since the behaviour
>>> doesn't consistently abstract over the concrete sequence types?  If code
>>> doesn't want to worry about what sequence is being used, then it has to
>>> manually check the return value or if a signal was raised, depending on
>>> the type of the argument to seq-last (which is to ignore the issue that
>>> we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
>>> the same also being the case for seq-last).
>>
>> It would be good to look systematically at what errors seq.el can
>> signal.  But it also seems that in practice the main value of seq.el is
>> to provide a bunch of handy functions rather than allowing you to work
>> with a sequence whose type you don't know.
>
> Yea, this has been my experience as well.  Usually I know what the
> sequence is, but seq provides a nice interface to not have to *remember*
> how to do what I need.  Just seq-do it and it will work somehow.

Another thing that should be kept in mind that sequences can be streams
(as provided by stream.el), and there doesn't have to be a final
element.

-- 
Philip Kaludercic

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Augusto Stoffel]

On Tue, 14 Mar 2023 at 22:04, Philip Kaludercic wrote:

>>> Maybe it's more useful to allow negative arguments in seq-elt?  Saying
>>> (seq-elt seq -1) isn't much more effort than (seq-last seq).
>>
>> I'm personally a bit ambiguous about negative indices.  It's nice for
>> those one liners when you need them, but they are quite confusing
>> especially if you switch between languages and they all implement them a
>> bit differently.
>
> The only language I am really familiar with is python, and what that
> effectively does is (mod i (length n)), what do other languages do?

In Python "abc"[-4] throws and error.  I don't think there's any choice
to be made here: If it were to support a negative index n, then
(seq-elt s n) should just return the (+ (length s) n)-th element, with
the usual treatment for out of bounds indices (whatever it is).

> Another thing that should be kept in mind that sequences can be streams
> (as provided by stream.el), and there doesn't have to be a final
> element.

What is the seq-length of an infinite stream?  And does asking for the
length of a stream consume it?  If so, then seq is an imperfect
abstraction for streams.

Re: [PATCH] seq.el: add seq-last for symmetry with seq-first

[Philip Kaludercic]

Augusto Stoffel <arstoffel@gmail.com> writes:

> On Tue, 14 Mar 2023 at 22:04, Philip Kaludercic wrote:
>
>>>> Maybe it's more useful to allow negative arguments in seq-elt?  Saying
>>>> (seq-elt seq -1) isn't much more effort than (seq-last seq).
>>>
>>> I'm personally a bit ambiguous about negative indices.  It's nice for
>>> those one liners when you need them, but they are quite confusing
>>> especially if you switch between languages and they all implement them a
>>> bit differently.
>>
>> The only language I am really familiar with is python, and what that
>> effectively does is (mod i (length n)), what do other languages do?
>
> In Python "abc"[-4] throws and error.  I don't think there's any choice
> to be made here: If it were to support a negative index n, then
> (seq-elt s n) should just return the (+ (length s) n)-th element, with
> the usual treatment for out of bounds indices (whatever it is).
>
>> Another thing that should be kept in mind that sequences can be streams
>> (as provided by stream.el), and there doesn't have to be a final
>> element.
>
> What is the seq-length of an infinite stream?  And does asking for the
> length of a stream consume it?  If so, then seq is an imperfect
> abstraction for streams.

Yes it does, since `seq' requires `seq-length' to be implemented for
every new sequence:

--8<---------------cut here---------------start------------->8---
(cl-defmethod seq-length ((stream stream))
  "Return the length of STREAM.
This function will eagerly consume the entire stream."
  (let ((len 0))
    (while (not (stream-empty-p stream))
      (setq len (1+ len))
      (setq stream (stream-rest stream)))
    len))
--8<---------------cut here---------------end--------------->8---

The only alternative I see here would be that streams raise a signal, if
you try to determine their length (or rather only if they are infinite,
which I don't know if you can determine without eager evaluation).