GNU bug report logs - #38576
[PATCH] gnu: r-irkernel: Fix R kernel loading

Previous Next

Package: guix-patches;

Reported by: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>

Date: Thu, 12 Dec 2019 09:29:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38576 in the body.
You can then email your comments to 38576 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 12 Dec 2019 09:29:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Lars-Dominik Braun <ldb <at> leibniz-psychology.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 12 Dec 2019 09:29:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 12 Dec 2019 08:46:13 +0100
* gnu/packages/cran.scm (r-irkernel): Absolute path to R binary
[propagated-inputs]: Generate proper search paths by adding r-minimal
---
 gnu/packages/cran.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/packages/cran.scm b/gnu/packages/cran.scm
index 765747ea3b..c54a076014 100644
--- a/gnu/packages/cran.scm
+++ b/gnu/packages/cran.scm
@@ -12414,6 +12414,12 @@ running IRkernel session.")
                        "--name" "ir"
                        "--prefix" out
                        (string-append out "/site-library/IRkernel/kernelspec"))
+               ;; Record the absolute file name of the 'R' executable in
+               ;; 'kernel.json'.
+               (substitute* (string-append out "/share/jupyter"
+                                           "/kernels/ir/kernel.json")
+                 (("\\[\"R\",")
+                  (string-append "[\"" (which "R") "\",")))
                #t))))))
     (inputs
      `(("jupyter" ,jupyter)))
@@ -12423,6 +12429,8 @@ running IRkernel session.")
        ("r-evaluate" ,r-evaluate)
        ("r-irdisplay" ,r-irdisplay)
        ("r-jsonlite" ,r-jsonlite)
+       ;; sets R_LIBS_SITE, so R can actually find this package (IRkernel)
+       ("r-minimal" ,r-minimal)
        ("r-pbdzmq" ,r-pbdzmq)
        ("r-repr" ,r-repr)
        ("r-uuid" ,r-uuid)))
-- 
2.20.1





Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 12 Dec 2019 09:42:01 GMT) Full text and rfc822 format available.

Message #8 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 12 Dec 2019 10:41:24 +0100
Hi Lars-Dominik,

> * gnu/packages/cran.scm (r-irkernel): Absolute path to R binary
> [propagated-inputs]: Generate proper search paths by adding r-minimal
> ---
>  gnu/packages/cran.scm | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gnu/packages/cran.scm b/gnu/packages/cran.scm
> index 765747ea3b..c54a076014 100644
> --- a/gnu/packages/cran.scm
> +++ b/gnu/packages/cran.scm
> @@ -12414,6 +12414,12 @@ running IRkernel session.")
>                         "--name" "ir"
>                         "--prefix" out
>                         (string-append out "/site-library/IRkernel/kernelspec"))
> +               ;; Record the absolute file name of the 'R' executable in
> +               ;; 'kernel.json'.
> +               (substitute* (string-append out "/share/jupyter"
> +                                           "/kernels/ir/kernel.json")
> +                 (("\\[\"R\",")
> +                  (string-append "[\"" (which "R") "\",")))
>                 #t))))))
>      (inputs
>       `(("jupyter" ,jupyter)))

This part looks fine to me, though I wonder if that’s what users of this
package would expect.  Is there an expectation that the effective R is
defined by the environment?  Or would that not work anyway?

> @@ -12423,6 +12429,8 @@ running IRkernel session.")
>         ("r-evaluate" ,r-evaluate)
>         ("r-irdisplay" ,r-irdisplay)
>         ("r-jsonlite" ,r-jsonlite)
> +       ;; sets R_LIBS_SITE, so R can actually find this package (IRkernel)
> +       ("r-minimal" ,r-minimal)
>         ("r-pbdzmq" ,r-pbdzmq)
>         ("r-repr" ,r-repr)
>         ("r-uuid" ,r-uuid)))

This doesn’t look right to me.  It seems wrong for any R package to
propagate R itself.  The R_LIBS_SITE variable is “attached” to
“r-minimal”, so when that is installed R will find the r-irkernel
package.  Am I missing something?

--
Ricardo





Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 12 Dec 2019 12:36:01 GMT) Full text and rfc822 format available.

Message #11 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 12 Dec 2019 11:04:52 +0100
Hi Ricardo,

> This part looks fine to me, though I wonder if that’s what users of this
> package would expect.  Is there an expectation that the effective R is
> defined by the environment?  Or would that not work anyway?
it’s what python-ipykernel does – without explanation though. I’m not an R
expert, so I’m unsure whether any R installation from the environment (which
could be user-installed in $HOME) would be able to load this plugin or just the
one it was “built” for. This change assumes the latter.

> 
> > @@ -12423,6 +12429,8 @@ running IRkernel session.")
> >         ("r-evaluate" ,r-evaluate)
> >         ("r-irdisplay" ,r-irdisplay)
> >         ("r-jsonlite" ,r-jsonlite)
> > +       ;; sets R_LIBS_SITE, so R can actually find this package (IRkernel)
> > +       ("r-minimal" ,r-minimal)
> >         ("r-pbdzmq" ,r-pbdzmq)
> >         ("r-repr" ,r-repr)
> >         ("r-uuid" ,r-uuid)))
> 
> This doesn’t look right to me.  It seems wrong for any R package to
> propagate R itself.  The R_LIBS_SITE variable is “attached” to
> “r-minimal”, so when that is installed R will find the r-irkernel
> package.  Am I missing something?
If r-minimal is not installed, the kernel will simply not work and thus
render this package useless. That’s why I would consider it a dependency.

Cheers,
Lars





Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 12 Dec 2019 19:15:01 GMT) Full text and rfc822 format available.

Message #14 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 12 Dec 2019 20:13:57 +0100
Hi,

Thank you for your patch!

On Thu, 12 Dec 2019 at 13:36, Lars-Dominik Braun
<ldb <at> leibniz-psychology.org> wrote:

> > > @@ -12423,6 +12429,8 @@ running IRkernel session.")
> > >         ("r-evaluate" ,r-evaluate)
> > >         ("r-irdisplay" ,r-irdisplay)
> > >         ("r-jsonlite" ,r-jsonlite)
> > > +       ;; sets R_LIBS_SITE, so R can actually find this package (IRkernel)
> > > +       ("r-minimal" ,r-minimal)
> > >         ("r-pbdzmq" ,r-pbdzmq)
> > >         ("r-repr" ,r-repr)
> > >         ("r-uuid" ,r-uuid)))
> >
> > This doesn’t look right to me.  It seems wrong for any R package to
> > propagate R itself.  The R_LIBS_SITE variable is “attached” to
> > “r-minimal”, so when that is installed R will find the r-irkernel
> > package.  Am I missing something?

> If r-minimal is not installed, the kernel will simply not work and thus
> render this package useless. That’s why I would consider it a dependency.

Hum? it is true for any R package, isn't it?
I mean, all the R packages needs R to work but R is not a dependency.

We can discuss if a R package has to propagate R itself or not.
But it is not how the R packages are usually defined in Guix; AFAIU.
To stay coherent, "r-minimal" should not be propagated or I also miss
something. :-)

Currently, I usually type "guix install r r-pkg" and not "guix install r-pkg".

All the best,
simon




Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 19 Dec 2019 22:48:01 GMT) Full text and rfc822 format available.

Message #17 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 19 Dec 2019 23:47:33 +0100
Hello,

Lars-Dominik Braun <ldb <at> leibniz-psychology.org> skribis:

>> This part looks fine to me, though I wonder if that’s what users of this
>> package would expect.  Is there an expectation that the effective R is
>> defined by the environment?  Or would that not work anyway?
> it’s what python-ipykernel does – without explanation though. I’m not an R
> expert, so I’m unsure whether any R installation from the environment (which
> could be user-installed in $HOME) would be able to load this plugin or just the
> one it was “built” for. This change assumes the latter.
>
>> 
>> > @@ -12423,6 +12429,8 @@ running IRkernel session.")
>> >         ("r-evaluate" ,r-evaluate)
>> >         ("r-irdisplay" ,r-irdisplay)
>> >         ("r-jsonlite" ,r-jsonlite)
>> > +       ;; sets R_LIBS_SITE, so R can actually find this package (IRkernel)
>> > +       ("r-minimal" ,r-minimal)
>> >         ("r-pbdzmq" ,r-pbdzmq)
>> >         ("r-repr" ,r-repr)
>> >         ("r-uuid" ,r-uuid)))
>> 
>> This doesn’t look right to me.  It seems wrong for any R package to
>> propagate R itself.  The R_LIBS_SITE variable is “attached” to
>> “r-minimal”, so when that is installed R will find the r-irkernel
>> package.  Am I missing something?
> If r-minimal is not installed, the kernel will simply not work and thus
> render this package useless. That’s why I would consider it a dependency.

An argument in favor of the status quo would be that it allows users to
choose between ‘r’ and ‘r-minimal’.  Is that a compelling argument?

It may be more important for ‘r-irkernel’ to work out of the box, like
you did.

However, if we go that route, we should arrange to not propagate
‘r-minimal’ (it’s intrusive) and instead have ‘kernel.json’ do the right
thing.

How does that sound?

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 02 Jan 2020 07:36:01 GMT) Full text and rfc822 format available.

Message #20 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 2 Jan 2020 08:35:36 +0100
Hi Ludo,

> An argument in favor of the status quo would be that it allows users to
> choose between ‘r’ and ‘r-minimal’.  Is that a compelling argument?
reading the documentation I thought this was possible using
--with-input=r-minimal=r ?

> However, if we go that route, we should arrange to not propagate
> ‘r-minimal’ (it’s intrusive) and instead have ‘kernel.json’ do the right
> thing.
I’m not following, sorry. What do you suggest kernel.json should do?

Lars





Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Thu, 02 Jan 2020 11:44:02 GMT) Full text and rfc822 format available.

Message #23 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Thu, 02 Jan 2020 12:43:51 +0100
Hi,

Lars-Dominik Braun <ldb <at> leibniz-psychology.org> skribis:

>> An argument in favor of the status quo would be that it allows users to
>> choose between ‘r’ and ‘r-minimal’.  Is that a compelling argument?
> reading the documentation I thought this was possible using
> --with-input=r-minimal=r ?

Yes, good point.

>> However, if we go that route, we should arrange to not propagate
>> ‘r-minimal’ (it’s intrusive) and instead have ‘kernel.json’ do the right
>> thing.
> I’m not following, sorry. What do you suggest kernel.json should do?

I was suggesting hard-coding the file name of the ‘R’ executable in
‘kernel.json’, but I see you already did that in your initial patch.
Sorry for the confusion.

On second thought, I think propagating R is acceptable in this case
because a Jupyter kernel is a thin wrapper around a programming language
implementation.

Unless there are objections, I’ll apply your initial patch.

Apologies for the delay, but I think it was good to have this
discussion!

Thanks,
Ludo’.




Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 03 Jan 2020 15:14:01 GMT) Full text and rfc822 format available.

Notification sent to Lars-Dominik Braun <ldb <at> leibniz-psychology.org>:
bug acknowledged by developer. (Fri, 03 Jan 2020 15:14:01 GMT) Full text and rfc822 format available.

Message #28 received at 38576-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576-done <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Fri, 03 Jan 2020 16:12:52 +0100
Ludovic Courtès <ludo <at> gnu.org> skribis:

> On second thought, I think propagating R is acceptable in this case
> because a Jupyter kernel is a thin wrapper around a programming language
> implementation.
>
> Unless there are objections, I’ll apply your initial patch.

Done!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Mon, 06 Jan 2020 18:15:02 GMT) Full text and rfc822 format available.

Message #31 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org,
 Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Mon, 6 Jan 2020 19:14:38 +0100
Hi,

On Thu, 2 Jan 2020 at 12:44, Ludovic Courtès <ludo <at> gnu.org> wrote:

> On second thought, I think propagating R is acceptable in this case
> because a Jupyter kernel is a thin wrapper around a programming language
> implementation.

So, the argument is "IRkernal is not an ordinary R package" and
because this very package is special it "is acceptable" to propagate R
(r-minimal).

No objection, but adding this explanation or a link to this thread
appears to me welcome. :-)
For example in the commit message or perhaps better a comment in
propagated-inputs.

Thank you for this package. Nice to have it! :-)

All the best,
simon




Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Tue, 07 Jan 2020 21:03:02 GMT) Full text and rfc822 format available.

Message #34 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>, Lars-Dominik Braun
 <ldb <at> leibniz-psychology.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Tue, 07 Jan 2020 22:02:13 +0100
On Thu, 2020-01-02 at 12:43 +0100, Ludovic Courtès wrote:
> Hi,
> 
> Lars-Dominik Braun <ldb <at> leibniz-psychology.org> skribis:
> 
> > > An argument in favor of the status quo would be that it allows users to
> > > choose between ‘r’ and ‘r-minimal’.  Is that a compelling argument?
> > reading the documentation I thought this was possible using
> > --with-input=r-minimal=r ?
> 
> Yes, good point.
> 
> > > However, if we go that route, we should arrange to not propagate
> > > ‘r-minimal’ (it’s intrusive) and instead have ‘kernel.json’ do the right
> > > thing.
> > I’m not following, sorry. What do you suggest kernel.json should do?
> 
> I was suggesting hard-coding the file name of the ‘R’ executable in
> ‘kernel.json’, but I see you already did that in your initial patch.
> Sorry for the confusion.
> 
> On second thought, I think propagating R is acceptable in this case
> because a Jupyter kernel is a thin wrapper around a programming language
> implementation.
> 
> Unless there are objections, I’ll apply your initial patch.
> 

I'm too late, but doesn't this break the kernel for people who have the "full" R
in their profile, and therefore expect the "full" R to be available in a Jupyter
notebook?

Kind regards,
Roel Janssen






Information forwarded to guix-patches <at> gnu.org:
bug#38576; Package guix-patches. (Tue, 07 Jan 2020 23:02:02 GMT) Full text and rfc822 format available.

Message #37 received at 38576 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Roel Janssen <roel <at> gnu.org>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 38576 <at> debbugs.gnu.org,
 Lars-Dominik Braun <ldb <at> leibniz-psychology.org>
Subject: Re: [bug#38576] [PATCH] gnu: r-irkernel: Fix R kernel loading
Date: Wed, 08 Jan 2020 00:01:26 +0100
Hi Roel,

Roel Janssen <roel <at> gnu.org> skribis:

> On Thu, 2020-01-02 at 12:43 +0100, Ludovic Courtès wrote:
>> Hi,
>> 
>> Lars-Dominik Braun <ldb <at> leibniz-psychology.org> skribis:
>> 
>> > > An argument in favor of the status quo would be that it allows users to
>> > > choose between ‘r’ and ‘r-minimal’.  Is that a compelling argument?
>> > reading the documentation I thought this was possible using
>> > --with-input=r-minimal=r ?
>> 
>> Yes, good point.
>> 
>> > > However, if we go that route, we should arrange to not propagate
>> > > ‘r-minimal’ (it’s intrusive) and instead have ‘kernel.json’ do the right
>> > > thing.
>> > I’m not following, sorry. What do you suggest kernel.json should do?
>> 
>> I was suggesting hard-coding the file name of the ‘R’ executable in
>> ‘kernel.json’, but I see you already did that in your initial patch.
>> Sorry for the confusion.
>> 
>> On second thought, I think propagating R is acceptable in this case
>> because a Jupyter kernel is a thin wrapper around a programming language
>> implementation.
>> 
>> Unless there are objections, I’ll apply your initial patch.
>> 
>
> I'm too late, but doesn't this break the kernel for people who have the "full" R
> in their profile, and therefore expect the "full" R to be available in a Jupyter
> notebook?

Indeed, it forces ‘r-minimal’ for use in the kernel.  But there are
workarounds, as Lars-Dominik suggests above.

WDYT?

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 05 Feb 2020 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 54 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.