From: "Pascal J. Bourguignon" <pjb@informatimago.com>
To: help-gnu-emacs@gnu.org
Subject: Re: simple first emacs script
Date: Wed, 15 Dec 2010 15:51:07 +0100 [thread overview]
Message-ID: <87hbefytr8.fsf@kuiper.lan.informatimago.com> (raw)
In-Reply-To: 2V2Oo.37337$hW6.28446@newsfe08.ams2
Tom <tom@somewhere.com> 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 {}.
next prev parent reply other threads:[~2010-12-15 14:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-15 12:55 simple first emacs script Tom
2010-12-15 14:51 ` Pascal J. Bourguignon [this message]
2010-12-15 17:28 ` Tom
2010-12-15 18:37 ` Pascal J. Bourguignon
2010-12-16 15:21 ` Tom
2010-12-16 21:55 ` Stefan Monnier
2010-12-17 22:13 ` Thien-Thi Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87hbefytr8.fsf@kuiper.lan.informatimago.com \
--to=pjb@informatimago.com \
--cc=help-gnu-emacs@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.