From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id iDNpDoFs0l4kQwAA0tVLHw (envelope-from ) for ; Sat, 30 May 2020 14:24:01 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id 6MfNCYFs0l76dgAA1q6Kng (envelope-from ) for ; Sat, 30 May 2020 14:24:01 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 575C1940BD0 for ; Sat, 30 May 2020 14:24:00 +0000 (UTC) Received: from localhost ([::1]:37544 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jf2P1-0008Km-1P for larch@yhetil.org; Sat, 30 May 2020 10:23:59 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58796) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jf2Of-0008Iz-Hd for emacs-orgmode@gnu.org; Sat, 30 May 2020 10:23:37 -0400 Received: from confino.investici.org ([2a00:c38:11e:ffff::a020]:60667) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jf2Oc-0000MA-OR for emacs-orgmode@gnu.org; Sat, 30 May 2020 10:23:37 -0400 Received: from mx1.investici.org (unknown [127.0.0.1]) by confino.investici.org (Postfix) with ESMTP id B59F521071; Sat, 30 May 2020 14:23:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=anche.no; s=stigmate; t=1590848610; bh=KFbS5g9WYOIBVLYSgak+WwtjkPby+ihJdLGro9sniS8=; h=Subject:To:References:From:Date:In-Reply-To:From; b=KkOQw1BtZ00whjHonFYs2QLiBZYVlv/MmXikreryUr3ZpMJ7LyWnwXRpBq97XGhBL ykYSrCgJIX3J7taotRhfBs0XZ4cuoZOs3hZCODPV+5xn6pvFywMjROM3+eQG7pd2BI MxdLrdM9O4hHEUuu1zHO23LF+dCmUnxwCgRGEvhQ= Received: from [212.103.72.250] (mx1.investici.org [212.103.72.250]) (Authenticated sender: mariotomo@inventati.org) by localhost (Postfix) with ESMTPSA id 8F4A421215; Sat, 30 May 2020 14:23:29 +0000 (UTC) Subject: Re: `with` as a list. To: Kyle Meyer , emacs-orgmode@gnu.org References: <656b815f-9e38-e68c-b7a3-091f6f3d36bf@anche.no> <87tuzyueli.fsf@kyleam.com> From: Mario Frasca Message-ID: Date: Sat, 30 May 2020 09:23:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: <87tuzyueli.fsf@kyleam.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Received-SPF: pass client-ip=2a00:c38:11e:ffff::a020; envelope-from=mario@anche.no; helo=confino.investici.org X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=anche.no header.s=stigmate header.b=KkOQw1Bt; dmarc=none; spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Spam-Score: -1.21 X-TUID: 0kQBEnzh1j4G Hi Kyle, thank you for writing back, I have a couple of questions in reply. btw. are you on irc.freenode.net?  I'm `mariotomo' there.  and I just joined `#org-mode'.  I don't think that my terminal will ring a bell if I'm mentioned there. On 30/05/2020 01:22, Kyle Meyer wrote: > Mario Frasca writes: > >> […] > Thanks for the patch and for clearly describing the motivation. I'm not > an org-plot user, but the change sounds useful. (It'd be great if > org-plot users could chime in.) thank you for taking the time to review! > Two meta-comments: > > * Please provide a patch with a commit message. See > for more information. > > * The link above also explains the copyright assignment requirement > for contributions. Please consider assigning copyright to the FSF. I'm editing in my cloned repository, and committing often, so I do not have a single commit, consequently also not a single commit message. I just sent my form request to assign@gnu.org. >> diff --git a/lisp/org-plot.el b/lisp/org-plot.el >> index a23195d2a..87a415137 100644 >> --- a/lisp/org-plot.el >> +++ b/lisp/org-plot.el >> @@ -179,6 +179,28 @@ and dependent variables." >> (setf back-edge "") (setf front-edge "")))) >> row-vals)) >> >> +(defun org-plot/zip-deps-with (num-cols ind deps with) >> + "describe each column to be plotted as (col . with)" > This doesn't match the convention used in this code base for docstrings. > Taking a look around should give you a good sense, but (1) the first > word should be capitalized, (2) sentences should end with a period, and > (3) ideally all parameters should be described in the docstring. ok, 1 and 2 are just consequence of my usual way of typing, however wrong, I will fix them.  3 I didn't consider.  useful point. > >> + ;; make 'deps explicit > I think this and the other comments in this function could safely be > dropped. :+1: >> + (unless deps >> + (setf deps (let (r) >> + (dotimes (i num-cols r) >> + (unless (eq num-cols (+ ind i)) >> + (setq r (cons (- num-cols i) r))))))) > […] using setq unless setf is needed would be more > consistent with this code base. the "unless needed" makes me suspect I should read what's the difference. > The code above looks fine to me. Another option would be to use > number-sequence and then filter out the ind value. no, that would break functionality: I need to keep the given order. btw my patch allows you to use the same column more than once. > >> + ;; invoke zipping function on converted data >> + (org-plot/zip deps with)) >> + >> +(defun org-plot/zip (xs ys) >> + (unless >> + (null xs) >> + (cons (cons (car xs) (or (car ys) "lines")) > Is the "lines" fall back ever reached? It looks like you're extending > `with' above to be the same length as `deps`. it is needed: I'm not extending any `with' given as a non-empty list. but I should be using some settings, some global default `with' value, which I don't know where to find.  hard coding "lines" can't be the right thing to do. >> + (org-plot/zip (cdr xs) (cdr ys))))) > In Elisp, it's not very common to use recursive functions (and there's > no TCO). In the case of zipping two lists, I think it'd probably be > easiest to just use cl-loop. And in my view having a separate function > here is probably an overkill. I'm not sure about the TCO (in other words: I haven't the faintest idea what you mean by that), and I would not know how to write this using `cl-loop'.  I'll have a look though. >> (defun org-plot/gnuplot-script (data-file num-cols params &optional preface) >> "Write a gnuplot script to DATA-FILE respecting the options set in PARAMS. >> NUM-COLS controls the number of columns plotted in a 2-d plot. >> @@ -240,22 +262,22 @@ manner suitable for prepending to a user-specified script." >> "%Y-%m-%d-%H:%M:%S") "\""))) >> (unless preface >> (pcase type ; plot command >> - (`2d (dotimes (col num-cols) >> - (unless (and (eq type '2d) >> - (or (and ind (equal (1+ col) ind)) >> - (and deps (not (member (1+ col) deps))))) >> - (setf plot-lines >> - (cons >> - (format plot-str data-file >> - (or (and ind (> ind 0) >> - (not text-ind) >> - (format "%d:" ind)) "") >> - (1+ col) >> - (if text-ind (format ":xticlabel(%d)" ind) "") >> - with >> - (or (nth col col-labels) >> - (format "%d" (1+ col)))) >> - plot-lines))))) >> + (`2d (dolist >> + (col-with >> + (org-plot/zip-deps-with num-cols ind deps with)) >> + (setf plot-lines >> + (cons >> + (format plot-str data-file >> + (or (and ind (> ind 0) >> + (not text-ind) >> + (format "%d:" ind)) "") >> + (car col-with) >> + (if text-ind (format ":xticlabel(%d)" ind) "") >> + (cdr col-with) >> + (or (nth (1- (car col-with)) >> + col-labels) >> + (format "%d" (car col-with)))) >> + plot-lines)))) > I haven't looked at this bit too closely (and I know the handling around > col-labels changed a bit in the last message you sent), but I did try > out a few org-plot invocations and things seemed to work as I expected. > I'll plan to take a closer when you send a reroll. the whole org-plot.el has (had) no unit tests, so if you share a couple of your org-plot invocations, I can convert them to regression tests. >> @@ -320,7 +343,7 @@ line directly before or after the table." >> 0) >> (plist-put params :timeind t) >> ;; Check for text ind column. >> - (if (or (string= (plist-get params :with) "hist") >> + (if (or (and (stringp with) (string= with "hist")) > Could be simplified by using `equal'. yes, definitely!  as said, I'm not a lisp programmer.  ;-) I hope to post a new diff soon. cheers, MF