From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: =?utf-8?Q?Ludovic_Court=C3=A8s?= Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Add srfi-171 to guile Date: Sun, 08 Mar 2020 15:40:59 +0100 Message-ID: <87fteic3k4.fsf@gnu.org> References: <87mub2ywcc.fsf@pobox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="108237"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) Cc: Guile Devel To: Linus =?utf-8?Q?Bj=C3=B6rnstam?= Original-X-From: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Sun Mar 08 15:41:13 2020 Return-path: Envelope-to: guile-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jAx7B-000S2t-8Q for guile-devel@m.gmane-mx.org; Sun, 08 Mar 2020 15:41:13 +0100 Original-Received: from localhost ([::1]:58304 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jAx79-0007y4-UI for guile-devel@m.gmane-mx.org; Sun, 08 Mar 2020 10:41:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60932) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jAx70-0007xy-8H for guile-devel@gnu.org; Sun, 08 Mar 2020 10:41:03 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:54350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1jAx70-0003bq-3a; Sun, 08 Mar 2020 10:41:02 -0400 Original-Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=35484 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jAx6z-0001nS-ID; Sun, 08 Mar 2020 10:41:01 -0400 X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 19 =?utf-8?Q?Vent=C3=B4se?= an 228 de la =?utf-8?Q?R?= =?utf-8?Q?=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu In-Reply-To: ("Linus \=\?utf-8\?Q\?Bj\=C3\=B6rnstam\=22's\?\= message of "Thu, 16 Jan 2020 20:52:33 +0100") X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Original-Sender: "guile-devel" Xref: news.gmane.io gmane.lisp.guile.devel:20438 Archived-At: Hi Linus, Linus Bj=C3=B6rnstam skribis: > From c382d7808a8d41cd4e9ef8a17b7ba9553835499b Mon Sep 17 00:00:00 2001 > From: =3D?UTF-8?q?Linus=3D20Bj=3DC3=3DB6rnstam?=3D > Date: Thu, 16 Jan 2020 20:31:45 +0100 > Subject: [PATCH] Add SRFI-171 (transducers) to guile. > > The two guile-specific additions are powerful transducers which can be > used to generalize transducers like tsegment. They are hard to get > right, but powerful and useful enough to warrant inclusion. > > * doc/ref/srfi-modules.texi: added srfi-171 section > * module/Makefile.am (SOURCES): > * module/srfi/srfi-171.scm: > * module/srfi/srfi-171/meta.scm: Add SRFI-171 > * module/srfi/srfi-171/gnu.scm: Add 2 guile-specific extensions. > * test-suite/Makefile.am (SCM_TESTS): > * test-suite/tests/srfi-171.test: Add tests. I think the patch is almost ready for inclusion, thanks for taking the time to address Andy=E2=80=99s comments! I have additional stylistic comments, and then I think we=E2=80=99re ready = to go: > +Transducers are oblivious to what kind of process they are used in, and > +are composable without building intermediate collections. This means we > +can create a transducer that squares all even numbers: @code{(compose > +(tfilter odd?) (tmap (lambda (x) (* x x))))} and reuse it with lists, For readability, it=E2=80=99s probably best to use @example rather than @co= de for the example above (for an example larger than a couple of words in general.) > +The central part of transducers are 3-arity reducing functions. In general, RnRS and Guile use the term =E2=80=9Cprocedure=E2=80=9D rather = than =E2=80=9Cfunctions=E2=80=9D. Not a big deal, but bonus points if you can a= djust the documentation accordingly. :-) > +@itemize > +@item > +(): Produce an identity s/an/the/ ? > +@subheading The concept of transducers > + > +A transducer is a one-arity function that takes a reducer and produces a > +reducing function that behaves as follows: > + > +@itemize > +@item > +(): calls reducer with no arguments (producing its identity) It=E2=80=99s not clear from this where you=E2=80=99d write () (there=E2=80= =99s a missing @code here, right?), which is not a valid Scheme expression in itself. Perhaps an extra bit of introduction is needed above for clarity? Also, this is under the =E2=80=9CConcept=E2=80=9D heading, but it looks mor= e like the API, no? > +@item > +(result-so-far): Maybe transform the result-so-far and call reducer with= it. > + > +@item > +(result-so-far input) Maybe do something to input and maybe call the > +reducer with result-so-far and the maybe-transformed input. Missing @code ornaments here. > +a simple example is as following: @code{ (list-transduce (tfilter odd?) ^ Capital. @example for the example. > ++ '(1 2 3 4 5))}. This first returns a transducer filtering all odd > +elements, then it runs @code{+} without arguments to retrieve its > +identity. It then starts the transduction by passing @code{+} to the Please make sure to add two spaces after end-of-sentence periods throughout the document. > + Even though transducers appear to be somewhat of a generalisation of > + map and friends, this is not really true. Since transducers don't know s/map/@code{map}/ > +@deffn {Scheme Procedure} list-transduce xform f lst > +@deffnx {Scheme Procedure} list-transduce xform f identity lst > + Initializes the transducer @code{xform} by passing the reducer @code{f} > + to it. If no identity is provided, @code{f} is run without arguments to > + return the reducer identity. It then reduces over @code{lst} using the > + identity as the seed. Please remove the extra leading space. Also, use @var, and use imperative tense, like so: @deffn {Scheme Procedure} list-transduce @var{xform} @var{f} @var{lst} Initialize the transducer @var{xform} by passing the reduce @var{f} to it. =E2=80=A6 @end deffn > +If one of the transducers finishes early (such as @code{ttake} or > +@code{tdrop}), it communicates this by returning a reduced value, which > +in the sample implementation is just a value wrapped in a SRFI 9 record > +type named "reduced". If such a value is returned by the transducer, For proper typesetting, write ``reduced'' instead of "reduced". s/SRFI /SRFI-/ > +Same as @code{list-transduce}, but for vectors, strings, u8-bytevectors > +and srfi-158-styled generators respectively. > + > +@end deffn Please remove the newline before @end deffn. > +@node SRFI-171 Helpers > +@subsubsection Helper functions for writing transducers > +@cindex transducers helpers > + > +These functions are in the @code{(srfi 171 meta)} module and are only ^ (srfi srfi-171 meta) > +@deffn {Scheme Procedure} reduced value > + > +Wraps a value in a @code{} container, signalling that the > +reduction should stop. > +@end deffn Please remove extra line before @deffn (throughout the document). > +(define-module (srfi srfi-171) > + #:declarative? #t Is it necessary? If so, could you add a comment so our future selves know whether this is still necessary? :-) > +(define reverse-rcons > + (case-lambda > + "A transducer-friendly consing reducer with '() as identity. > +The resulting list is in reverse order." In general, the style for docstrings is to use imperative tense, like: Return a consing reducer with the empty list as its identity. (Throughout the file.) Could you also add docstrings to the exported procedures that lack one? (Docstrings can be short of course, no need to be as precise as in the manual.) > +++ b/test-suite/tests/srfi-171.test > @@ -0,0 +1,195 @@ > +;; TODO: test all transducers that take an equality predicate > +;; TODO: test treplace with all kinds of hash tables Should we wait for these tests before merging? Tested code is always better than untested code. :-) Also, please add a Guile copyright header. > + (pass-if "tfilter+tmap" (equal? > + '(1 3 5) > + (list-transduce (compose (tfilter even?) (tma= p add1)) rcons numeric-list))) Please wrap lines to 80 chars. Could you send an updated patch? Thank you! Ludo=E2=80=99.