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?Linus_Bj=C3=B6rnstam?= Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Add srfi-171 to guile Date: Sun, 08 Mar 2020 18:11:17 +0100 Message-ID: <73b0e039-bf84-473d-bf50-5e63eec462e7@www.fastmail.com> References: <87mub2ywcc.fsf@pobox.com> <87fteic3k4.fsf@gnu.org> 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="110098"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Cyrus-JMAP/3.1.7-991-g5a577d3-fmstable-20200305v3 Cc: Guile Devel To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= Original-X-From: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Sun Mar 08 18:12:08 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 1jAzTD-000SV4-7M for guile-devel@m.gmane-mx.org; Sun, 08 Mar 2020 18:12:07 +0100 Original-Received: from localhost ([::1]:59778 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jAzTB-0008Rt-S9 for guile-devel@m.gmane-mx.org; Sun, 08 Mar 2020 13:12:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49203) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jAzSs-0008Rm-IS for guile-devel@gnu.org; Sun, 08 Mar 2020 13:11:48 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jAzSq-00055R-V8 for guile-devel@gnu.org; Sun, 08 Mar 2020 13:11:46 -0400 Original-Received: from wout1-smtp.messagingengine.com ([64.147.123.24]:42971) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jAzSq-0004zk-Gm; Sun, 08 Mar 2020 13:11:44 -0400 Original-Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id 227C25A4; Sun, 8 Mar 2020 13:11:41 -0400 (EDT) Original-Received: from imap1 ([10.202.2.51]) by compute7.internal (MEProxy); Sun, 08 Mar 2020 13:11:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=veryfast.biz; h= mime-version:message-id:in-reply-to:references:date:from:to:cc :subject:content-type:content-transfer-encoding; s=fm2; bh=2ndCz vgbHeRhw4ctyz2C6KJmzX158CSDYp+qjH1/1vs=; b=Riiao0ghPS/RfPCW/H8qm ocmOp4Buw57HNyBC1LhyWBq8NN17cgcYAP5OdGaFYW+2u7663umw0zv9DHa0iFLK c5GIBW71TBGnEWgNNpWowV39fDriEhWLDQFbvh7RuDNonk7SzyA2VY8UUJkfnZoe eTXYNWyZTBz2zbPzfU3WUCwtY5oQedRCicnTZoZ/nPWjomPKzADEUyN4I4ljGJn4 3SEaVkTrd6JXu2HRyo9KkabQ8/jC3LbRA/T3J5w1kPF54Af9OpvnP3+P9VtccNql n3JB5OLX2HMAz+1iMsNbi5EC1ebdptg6L4fxXH/UTx0M203bzedGgC3EGgp8O3oa w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=2ndCzvgbHeRhw4ctyz2C6KJmzX158CSDYp+qjH1/1 vs=; b=MQyV4xvxgBNtrX9WRT4drysxB//ewNnA4oZ/1VZk4rkbxRFU/PPcdnHPC ucRut8xcl4/1kx2SHFIwbNZUw1THoSMAMI/4/6wZfwpRwy4FhC3fVe0Cbja/+vGF Uo5+n4rocgEj2RdiPKBgYJRBnrEWFV8mv+FBQTLP+1ZlziGbRKD/gnQXY/C66+As HzQhOFGkpmKyD7B8lgPHj2szg9rlNfoN2BbvgpTfYAiBydUZviIAR50qkyCdn+Cc tL8+ND5XkkPY7Ls9pRorY+oeGsgASYqlvVUBwThzd58YcVbtPp5pZzKjFgMC84xv GZHvDwDKh/bHIVFRfAEBjeiB+fwAA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrudduiedguddttdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefofgggkfgjfhffhffvufgtgfesthhqredtreerjeenucfhrhhomhepnfhi nhhushgpuehjnphrnhhsthgrmhcuoehlihhnuhhsrdgsjhhorhhnshhtrghmsehvvghrhi hfrghsthdrsghiiieqnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghi lhhfrhhomheplhhinhhushdrsghjohhrnhhsthgrmhesvhgvrhihfhgrshhtrdgsihii X-ME-Proxy: Original-Received: by mailuser.nyi.internal (Postfix, from userid 501) id 41454C200A4; Sun, 8 Mar 2020 13:11:40 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface In-Reply-To: <87fteic3k4.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 64.147.123.24 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:20440 Archived-At: Thanks! I will address all those.=20 The tests are written and passing, so I will remove the redundant TODOs.= --=20 Linus Bj=C3=B6rnstam On Sun, 8 Mar 2020, at 15:40, Ludovic Court=C3=A8s wrote: > Hi Linus, >=20 > Linus Bj=C3=B6rnstam skribis: >=20 > > From c382d7808a8d41cd4e9ef8a17b7ba9553835499b Mon Sep 17 00:00:00 20= 01 > > 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. >=20 > I think the patch is almost ready for inclusion, thanks for taking the= > time to address Andy=E2=80=99s comments! >=20 > I have additional stylistic comments, and then I think we=E2=80=99re r= eady to go: >=20 > > +Transducers are oblivious to what kind of process they are used in,= and > > +are composable without building intermediate collections. This mean= s we > > +can create a transducer that squares all even numbers: @code{(compo= se > > +(tfilter odd?) (tmap (lambda (x) (* x x))))} and reuse it with list= s, >=20 > For readability, it=E2=80=99s probably best to use @example rather tha= n @code > for the example above (for an example larger than a couple of words in= > general.) >=20 > > +The central part of transducers are 3-arity reducing functions. >=20 > In general, RnRS and Guile use the term =E2=80=9Cprocedure=E2=80=9D ra= ther than > =E2=80=9Cfunctions=E2=80=9D. Not a big deal, but bonus points if you = can adjust the > documentation accordingly. :-) >=20 > > +@itemize > > +@item > > +(): Produce an identity >=20 > s/an/the/ ? >=20 > > +@subheading The concept of transducers > > + > > +A transducer is a one-arity function that takes a reducer and produ= ces a > > +reducing function that behaves as follows: > > + > > +@itemize > > +@item > > +(): calls reducer with no arguments (producing its identity) >=20 > 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? >=20 > Also, this is under the =E2=80=9CConcept=E2=80=9D heading, but it look= s more like the > API, no? >=20 > > +@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 th= e > > +reducer with result-so-far and the maybe-transformed input. >=20 > Missing @code ornaments here. >=20 > > +a simple example is as following: @code{ (list-transduce (tfilter o= dd?) > ^ > Capital. >=20 > @example for the example. >=20 > > ++ '(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 th= e >=20 > Please make sure to add two spaces after end-of-sentence periods > throughout the document. >=20 > > + Even though transducers appear to be somewhat of a generalisation = of > > + map and friends, this is not really true. Since transducers don't = know >=20 > s/map/@code{map}/ >=20 > > +@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 @co= de{f} > > + to it. If no identity is provided, @code{f} is run without argumen= ts to > > + return the reducer identity. It then reduces over @code{lst} using= the > > + identity as the seed. >=20 > Please remove the extra leading space. Also, use @var, and use > imperative tense, like so: >=20 > @deffn {Scheme Procedure} list-transduce @var{xform} @var{f} @var{ls= t} > Initialize the transducer @var{xform} by passing the reduce @var{f} = to > it. =E2=80=A6 > @end deffn >=20 > > +If one of the transducers finishes early (such as @code{ttake} or > > +@code{tdrop}), it communicates this by returning a reduced value, w= hich > > +in the sample implementation is just a value wrapped in a SRFI 9 re= cord > > +type named "reduced". If such a value is returned by the transducer= , >=20 > For proper typesetting, write ``reduced'' instead of "reduced". >=20 > s/SRFI /SRFI-/ >=20 > > +Same as @code{list-transduce}, but for vectors, strings, u8-bytevec= tors > > +and srfi-158-styled generators respectively. > > + > > +@end deffn >=20 > Please remove the newline before @end deffn. >=20 > > +@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 on= ly > ^ > (srfi srfi-171 meta) >=20 > > +@deffn {Scheme Procedure} reduced value > > + > > +Wraps a value in a @code{} container, signalling that the > > +reduction should stop. > > +@end deffn >=20 > Please remove extra line before @deffn (throughout the document). >=20 > > +(define-module (srfi srfi-171) > > + #:declarative? #t >=20 > Is it necessary? If so, could you add a comment so our future selves > know whether this is still necessary? :-) >=20 > > +(define reverse-rcons > > + (case-lambda > > + "A transducer-friendly consing reducer with '() as identity. > > +The resulting list is in reverse order." >=20 > In general, the style for docstrings is to use imperative tense, like:= >=20 > Return a consing reducer with the empty list as its identity. >=20 > (Throughout the file.) >=20 > 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.) >=20 > > +++ 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 >=20 > Should we wait for these tests before merging? Tested code is always > better than untested code. :-) >=20 > Also, please add a Guile copyright header. >=20 > > + (pass-if "tfilter+tmap" (equal? > > + '(1 3 5) > > + (list-transduce (compose (tfilter even?)= (tmap add1)) rcons numeric-list))) >=20 > Please wrap lines to 80 chars. >=20 > Could you send an updated patch? >=20 > Thank you! >=20 > Ludo=E2=80=99. >