Subject: Re: Submission of patch for Add r-abc

From: Ricardo Wurmus <[email-masked]>
Date: Wed, 21 Mar 2018 22:27:49 +0100

Hi Sahithi,

> I am submitting a patch for r-abc. Please review.

Thank you for the patch!

What follows is a short review that includes things I could change
myself before applying the patch, but for the sake of learning I
explicitly point out in this message.  I hope you don’t mind that I’m
focusing on what may seem like minor details.

> From 4b3b3eee4995616059d8a27489affc43cee26ea4 Mon Sep 17 00:00:00 2001
> From: Sahithi Yarlagadda <[email-masked]>
> Date: Wed, 21 Mar 2018 16:30:18 +0530
> Subject: [PATCH] gnu: Add r-abc.
>
>     * gnu/packages/cran.scm (r-abc): New variable.

Nitpick: please don’t indent this line.  The asterisk should be the
first character in this line.

> +
> +(define-public r-abc
> +  (package
> +    (name "r-abc")
> +    (version "2.1")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri (cran-uri "abc" version))
> +        (sha256
> +          (base32
> +            "0ngzaaz2y2s03fhngvwipmy4kq38xrmyddaz6a6l858rxvadrlhb"))))
> +    (build-system r-build-system)
> +    (propagated-inputs
> +      `(("r-abc-data" ,r-abc-data)
> +        ("r-locfit" ,r-locfit)
> +        ("r-mass" ,r-mass)
> +        ("r-nnet" ,r-nnet)
> +        ("r-quantreg" ,r-quantreg)))
> +    (home-page
> +      "http://cran.r-project.org/web/packages/abc")

Please pull this onto the previous line and use “https”.

> +    (synopsis
> +      "Tools for approximate bayesian computation (ABC)")

Please combine this line with the previous line.

> +    (description
> +      "Implements several ABC algorithms for performing parameter estimation,
> +model selection, and goodness-of-fit.  Cross-validation tools are also
> +available for measuring the accuracy of ABC estimates, and to calculate the
> +misclassification probabilities of different models.")

Please use full sentences.  For CRAN packages the official description
usually provides only a sentence fragment in place of the first
sentence.  Please also use “@dfn{Approximate Bayesian Computation}
(ABC)” the first time “ABC” is mentioned.

All thoughout the indentation is a bit off (that’s expected as the
importer is very simple and doesn’t know about the expected
indentation).  If you’re using Emacs you can easily adjust it with M-C-q
(also known as “indent-sexp”); otherwise please use
“etc/indent-code.el”.

Have you built this package with Guix?

Finally, please rebase this patch so that it can be applied
independently from the other patch you’ve sent, which cannot currently
be applied due to problems in building the defined package.  Rebasing
this patch ensures that it can be merged even if the other patch is
stuck.

Thanks!

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net