From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Pascal J. Bourguignon" Newsgroups: gmane.emacs.help Subject: Re: simple first emacs script Date: Wed, 15 Dec 2010 15:51:07 +0100 Organization: Informatimago Message-ID: <87hbefytr8.fsf@kuiper.lan.informatimago.com> References: <2V2Oo.37337$hW6.28446@newsfe08.ams2> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1292429357 22839 80.91.229.12 (15 Dec 2010 16:09:17 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 15 Dec 2010 16:09:17 +0000 (UTC) To: help-gnu-emacs@gnu.org Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Wed Dec 15 17:09:12 2010 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PStua-00056O-M2 for geh-help-gnu-emacs@m.gmane.org; Wed, 15 Dec 2010 17:09:04 +0100 Original-Received: from localhost ([127.0.0.1]:56690 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PStVw-0007WF-KD for geh-help-gnu-emacs@m.gmane.org; Wed, 15 Dec 2010 10:43:36 -0500 Original-Path: usenet.stanford.edu!fu-berlin.de!uni-berlin.de!individual.net!not-for-mail Original-Newsgroups: gnu.emacs.help Original-Lines: 397 Original-X-Trace: individual.net S5a2MEFNKlbFyo/3WY5FcgIlfhOtrE2LIg4sHThODSontyXXje Cancel-Lock: sha1:ZjAwYzE2YTIxOTYwNGIwMjEzMzNhODY3ZjZjYzk5NTMwYjAzYzQ0Yw== sha1:yFLQL58ecgd6+dzXjGkLiawMIjc= Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwAQMAAABtzGvEAAAABlBMVEUAAAD///+l2Z/dAAAA oElEQVR4nK3OsRHCMAwF0O8YQufUNIQRGIAja9CxSA55AxZgFO4coMgYrEDDQZWPIlNAjwq9 033pbOBPtbXuB6PKNBn5gZkhGa86Z4x2wE67O+06WxGD/HCOGR0deY3f9Ijwwt7rNGNf6Oac l/GuZTF1wFGKiYYHKSFAkjIo1b6sCYS1sVmFhhhahKQssRjRT90ITWUk6vvK3RsPGs+M1RuR mV+hO/VvFAAAAABJRU5ErkJggg== X-Accept-Language: fr, es, en X-Disabled: X-No-Archive: no User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) Original-Xref: usenet.stanford.edu gnu.emacs.help:183293 X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.help:77547 Archived-At: Tom writes: > I am about 2/3 way through the introduction to emacs lisp, and thought > it would help my learning if I wrote tried writing a simple script to > do something useful to me with the lessons I have covered so far so > far. I should point out I am surgeon not a computer scientist and > know nothing more about programming than what is contained in the > introduction to emacs lisp. > I end up with this: > > (defun nirs-data-clean (number-of-channels) > "Cleans the output files from the INVOS monitor removing all columns > apart from StO2 values, and the date and time stamps. Sto2 columns to > be retained are specified by the NUMBER-OF-CHANNELS variable. It > assumes that channels are always used in numerical order, i.e. channel > 1 always, then 2, then 3 then 4." > (interactive "nEnter number of channels (1-4): ") [...] First, unless the indenting was destroyed by your newsreader program, you might want to make use of the indent-region command ( type C-M-\ ) so that your source will look like: (defun nirs-data-clean (number-of-channels) "Cleans the output files from the INVOS monitor removing all columns apart from StO2 values, and the date and time stamps. Sto2 columns to be retained are specified by the NUMBER-OF-CHANNELS variable. It assumes that channels are always used in numerical order, i.e. channel 1 always, then 2, then 3 then 4." (interactive "nEnter number of channels (1-4): ") (barf-if-buffer-read-only) (require 'csv-mode) (push-mark (point-max)) (goto-char (point-min)) (if (or (eq number-of-channels 1) (eq number-of-channels 2) (eq number-of-channels 3) (eq number-of-channels 4)) (or (when (eq number-of-channels 1) (csv-kill-fields '(4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34) (point) (mark))) (when (eq number-of-channels 2) (csv-kill-fields '(4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34) (point) (mark))) (when (eq number-of-channels 3) (csv-kill-fields '(4 5 6 7 8 9 11 12 13 14 15 16 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34) (point) (mark))) (when (eq number-of-channels 4) (csv-kill-fields '(4 5 6 7 8 9 11 12 13 14 15 16 18 19 20 21 22 23 25 26 27 28 29 30 31 32 33 34) (point) (mark)) (message "%s is not a valid number of channels. Please enter a number between 1 and 4." number-of-channels)))) (when (y-or-n-p "Replace 0 StO$_2$ values with na: ") (progn (goto-char (point-min)) (while (re-search-forward "[^[:digit:]][0][^[:digit:]]" nil t) (replace-match "na "))))) Instead of calling barf-if-buffer-read-only, you can just prefix the interactive string with a star: (interactive "*nEnter number of channels (1-4): ") The (require 'csv-mode) form would be better placed on the toplevel (ie. above the defun form). Instead of push-mark (I don't see the matching pop-mark), you might use the save-excursion macro. Instead of using eq, you might prefer to use eql by default. In the case of numbers, it would be better to use =. Instead of: (or (when ... ...) (when ... ...) ...) you can write: (cond (... ...) (... ...) ...) And since all your conditions are about the same variable, matching a set of value comparable with eql, you could instead use case: (case number-of-channels ((1) ...) ((2) ...) ((3) ...) ((4) ...) (otherwise (error ...))) Notice how the indentation shows that your message form is not placed at the right level. You intended it to be the else branch of the if form, but you put it inside the last when body. Also you probably want to use error here instead of message, since if the number of channel is wrong, you probably don't want to replace the StO2 zeroes. You may want to use paredit, which helps editing lisp code in a structural way. http://www.emacswiki.org/ParEdit http://www.emacswiki.org/emacs/PareditCheatsheet http://mumble.net/~campbell/emacs/paredit.el Just download this paredit.el file, and put it in /usr/share/emacs/site-lisp/ (check that this path is in load-path, typing C-h v load-path RET. If it is not there, then use the path of the site-lisp directory listed in load-path), then put: (require 'cl) (require 'paredit) (push 'paredit-mode emacs-lisp-mode-hook) (push 'paredit-mode lisp-mode-hook) in your ~/.emacs file. If you want to activate immediately, without restarting emacs, you may reload ~/.emacs with M-x load-file RET ~/.emacs RET Finally, since the bodies of each case branch are the same, you may distribute it around and write: (csv-kill-fields (case number-of-channels ((1) '(4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34)) ((2) '(4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34)) ((3) '(4 5 6 7 8 9 11 12 13 14 15 16 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34)) ((4) '(4 5 6 7 8 9 11 12 13 14 15 16 18 19 20 21 22 23 25 26 27 28 29 30 31 32 33 34)) (otherwise (message "%s is not a valid number of channels. Please enter a number between 1 and 4." number-of-channels))) start end) But then, it might be clearer to give an intensional definition instead of an extensional one. A practical operator would be iota: (defun iota (count &optional start step) " RETURN: A list containing the elements (start start+step ... start+(count-1)*step) The start and step parameters default to 0 and 1, respectively. This procedure takes its name from the APL primitive. EXAMPLES: (iota 5) => (0 1 2 3 4) (iota 5 0 -0.1) => (0 -0.1 -0.2 -0.3 -0.4) " (setf start (or start 0) step (or step 1)) (when (< 0 count) (do ((result '()) (item (+ start (* step (1- count))) (- item step))) ((< item start) result) (push item result)))) So you could write: (csv-kill-fields (set-difference (iota 34 1) (ecase number-of-channels ((1) '(1 2 3)) ((2) '(1 2 3 10)) ((3) '(1 2 3 10 17)) ((4) '(1 2 3 10 17 24)))) (point-min) (point-max)) We may remove the otherwise branch, assuming we only get correct number-of-channels (and using ecase instead of case to signal an error if not), which we can ensure by using a more sophisticated interactive form, using competing-read: (interactive (list (first (read-from-string (completing-read "Enter number of channels (1-4): " (mapcar (lambda (x) (cons (format "%d" x) nil)) (iota 4 1)) (function identity) t ; require match "" nil "1" nil))))) (barf-if-buffer-read-only) ; in this case, we need to call it ; separately. Be sure to read the documentation of each operator used, eg. with: C-h f interactive RET C-h f completing-read RET etc. when (and unless and other macros) has an implicit progn, so there's no need to embed one in it. So to wrap it up so far, you could have: (defun nirs-data-clean (number-of-channels) "Cleans the output files from the INVOS monitor removing all columns apart from StO2 values, and the date and time stamps. Sto2 columns to be retained are specified by the NUMBER-OF-CHANNELS variable. It assumes that channels are always used in numerical order, i.e. channel 1 always, then 2, then 3 then 4." (interactive (list (first (read-from-string (completing-read "Enter number of channels (1-4): " (mapcar (lambda (x) (cons (format "%d" x) nil)) (iota 4 1)) (function identity) t ; require match "" nil "1" nil))))) (barf-if-buffer-read-only) (save-excursion (let ((start (point-min)) (end (point-max))) (goto-char start) (csv-kill-fields (set-difference (iota 34 1) (ecase number-of-channels ((1) '(1 2 3)) ((2) '(1 2 3 10)) ((3) '(1 2 3 10 17)) ((4) '(1 2 3 10 17 24)))) start end) (when (y-or-n-p "Replace 0 StO$_2$ values with na: ") (goto-char start) (while (re-search-forward "[^[:digit:]][0][^[:digit:]]" nil t) (replace-match "na ")))))) > The idea is to automatically clean unwanted columns from csv (space > separated) data file and ask me if I want to replace 0 values with > na. Ah, if you read the documentation of csv-kill-fields, you will see that it depends on the right setting of the variable csv-separators to know what separator to use. By default I have it set to a comma. So you want to bind this variable in your function: (let ((csv-separators '(" "))) (csv-kill-fields ...)) Note however that it is a single character string, and that your fields are separated by several. When I try it, it fails with csv-kill-fields complaining about the number of columns. It is probably better to use commas to separate the fields, so: (defun nirs-data-clean (number-of-channels) "Cleans the output files from the INVOS monitor removing all columns apart from StO2 values, and the date and time stamps. Sto2 columns to be retained are specified by the NUMBER-OF-CHANNELS variable. It assumes that channels are always used in numerical order, i.e. channel 1 always, then 2, then 3 then 4." (interactive (list (first (read-from-string (completing-read "Enter number of channels (1-4): " (mapcar (lambda (x) (cons (format "%d" x) nil)) (iota 4 1)) (function identity) t ; require match "" nil "1" nil))))) (barf-if-buffer-read-only) (save-excursion (replace-regexp " +" "," nil (point-min) (point-max)) (goto-char (point-min)) (let ((csv-separators '(","))) (csv-kill-fields (set-difference (iota 34 1) (ecase number-of-channels ((1) '(1 2 3)) ((2) '(1 2 3 10)) ((3) '(1 2 3 10 17)) ((4) '(1 2 3 10 17 24)))) (point-min) (point-max))) (when (y-or-n-p "Replace 0 StO$_2$ values with na: ") (goto-char (point-min)) (while (re-search-forward "[^[:digit:]]0[^[:digit:]]" nil t) (replace-match "na "))) ;; let's put back spaces as separators: (replace-regexp "," " " nil (point-min) (point-max)))) > I appreciate this is very messy, and I should be able to specify > ranges of field rather than listing every single one (but I couldn't > get that to work), but it works kind of. When I evaluate it and run > it for the first time it works, but if I undo the changes to the file > and run it again it often only takes out the 4th column, if I reopen > the file or re-evaluate the script it usually works again but I can't > seem to work the pattern of working and not working - as I can't see > the pattern I can't learn where I am going wrong. I see no reason why that would happen. In any case, my version seems to work correctly, even after undoing and running it again. > I assume I have made an obvious mistake but I don't have the > experience to know what it is. Any general comments about > simple/better ways to clean this script would be welcome, for example > would I be better making the regexp replace a yes/no arg to the > function so if I call this script from another script I can specify a > y/n argument (I don't know how to do this anyway though). Indeed, this is a good intension. You can separate interactive user interface commands from the functions doing the actual work, so that you can reuse the later. The interactive form allow you to merge both usages. You only have to add the wanted parameter, and specify it in the interactive form. You can also specify some parameters optional: (defun nirs-data-clean (number-of-channels &optional replace-StO2-0-p) ...) The simple interactive form would use a multiline string: (interactive "*nEnter number of channels (1-4): sReplace 0 StO$_2$ values with na (y/n): ") Unfortunately, interactive doesn't provide for a 'boolean' input, so we'll keep using the sophisticated form: (defun nirs-data-clean (number-of-channels &optional replace-StO2-0-p) "Cleans the output files from the INVOS monitor removing all columns apart from StO2 values, and the date and time stamps. Sto2 columns to be retained are specified by the NUMBER-OF-CHANNELS variable. It assumes that channels are always used in numerical order, i.e. channel 1 always, then 2, then 3 then 4." (interactive (list (first (read-from-string (completing-read "Enter number of channels (1-4): " (mapcar (lambda (x) (cons (format "%d" x) nil)) (iota 4 1)) (function identity) t ; require match "" nil "1" nil))) (string= "yes" (completing-read "Replace 0 StO$_2$ values with na: " (mapcar (lambda (x) (cons x nil)) '("yes" "no")) (function identity) t ; require match "" nil "yes" nil)))) (barf-if-buffer-read-only) (save-excursion (replace-regexp " +" "," nil (point-min) (point-max)) (goto-char (point-min)) (let ((csv-separators '(","))) (csv-kill-fields (set-difference (iota 34 1) (ecase number-of-channels ((1) '(1 2 3)) ((2) '(1 2 3 10)) ((3) '(1 2 3 10 17)) ((4) '(1 2 3 10 17 24)))) (point-min) (point-max))) (when replace-StO2-0-p (goto-char (point-min)) (while (re-search-forward "[^[:digit:]]0[^[:digit:]]" nil t) (replace-match "na "))) ;; let's put back spaces as separators: (replace-regexp "," " " nil (point-min) (point-max)))) Finally, when you'll have completed the "Introduction to Emacs Lisp", I would advise you to read SICP. SICP = Structure and Interpretation of Computer Programs http://mitpress.mit.edu/sicp/full-text/book/book-Z-H-4.html http://swiss.csail.mit.edu/classes/6.001/abelson-sussman-lectures/ http://www.codepoetics.com/wiki/index.php?title=Topics:SICP_in_other_languages http://eli.thegreenplace.net/category/programming/lisp/sicp/ http://www.neilvandyke.org/sicp-plt/ http://www.youtube.com/watch?v=rdj6deraQ6k (and watch the videos, they're quite instructive). While this book uses scheme, it's subject matter is not about scheme, but programming in general, as can be seen from the translations of the exercises in other programming languages (well, it seems the wiki has been defaced, so it's not available hopefully temporarily, but eli's blog contains Common Lisp versions of the exercises, which are close to emacs lisp). -- __Pascal Bourguignon__ http://www.informatimago.com/ A bad day in () is better than a good day in {}.