unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
@ 2021-04-21 18:29 Luciana Lima Brito
  2021-04-22  7:53 ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-21 18:29 UTC (permalink / raw)
  To: guix-devel, mail

Hi,

This email is mostly addressed to Christopher Baines, due to the
Outreachy.

In the last email to me, you said this about my next steps on
contributing:

"In terms of what to do next, you could continue on this derivation
comparison path. Some of the code you've got here could be used to make
the data better right when the database is queried. Take the recursive
field for outputs for example, it would be better to convert it to a
boolean where the database query is made."

I looked for something related to that in the project and I found
interesting stuff on comparison.scm, is this the relevant file to make
the data better? I could see that the queries are made in there.

For example, the test I used for the outputs field "recursive"
(recursive. ,(string=? recursive 't')) on controller.scm I moved to the
function derivation-outputs-differences-data on comparison.scm, and it
worked properly. Is this the kind of change I should be doing?

In case of a yes, which kind of improvements should I be aiming for?

Furthermore, should I try to achieve any improvements to the queries
itself, or this is not necessary?

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-21 18:29 Outreachy - Guix Data Service: questions about improving the data for derivation comparisons Luciana Lima Brito
@ 2021-04-22  7:53 ` Christopher Baines
  2021-04-22 20:00   ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-22  7:53 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
> This email is mostly addressed to Christopher Baines, due to the
> Outreachy.
>
> In the last email to me, you said this about my next steps on
> contributing:
>
> "In terms of what to do next, you could continue on this derivation
> comparison path. Some of the code you've got here could be used to make
> the data better right when the database is queried. Take the recursive
> field for outputs for example, it would be better to convert it to a
> boolean where the database query is made."
>
> I looked for something related to that in the project and I found
> interesting stuff on comparison.scm, is this the relevant file to make
> the data better? I could see that the queries are made in there.
>
> For example, the test I used for the outputs field "recursive"
> (recursive. ,(string=? recursive 't')) on controller.scm I moved to the
> function derivation-outputs-differences-data on comparison.scm, and it
> worked properly. Is this the kind of change I should be doing?
>
> In case of a yes, which kind of improvements should I be aiming for?
>
> Furthermore, should I try to achieve any improvements to the queries
> itself, or this is not necessary?

Small intentional changes are better, so I'd start just with looking at
some of the data coming out of the query. But yes, I think you're in the
right place. The hard part here is probably to look at how those values
are used in the JSON and HTML rendering code, and adjust that
accordingly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-22  7:53 ` Christopher Baines
@ 2021-04-22 20:00   ` Luciana Lima Brito
  2021-04-22 20:08     ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-22 20:00 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

Hi,

> Small intentional changes are better, so I'd start just with looking
> at some of the data coming out of the query. But yes, I think you're
> in the right place. The hard part here is probably to look at how
> those values are used in the JSON and HTML rendering code, and adjust
> that accordingly.

I made some modifications(moved the tests from controller.scm to
comparison.scm) in order to remove the value "#f" from the outputs
"hash-algorithm" and "hash" when rendering the html, for derivations
that don't have these values set, but doing that now I'm showing them
as empty fields on the json (instead of omitting them entirely).
This simplified a bit the processing on render-compare/derivations,
and I think the html output is also better. For inputs, sources, etc,
there are no processing being done on controller.scm other than the
map. I can't see now what else could be done in this regard.

An idea I had was to remove the entire map functions from the
controller.scm to comparison.scm, but this would impact the html, so I
would also have to modify the html, because the data format for json and
html are slightly different. I'm not sure if this is what you want.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-22 20:00   ` Luciana Lima Brito
@ 2021-04-22 20:08     ` Christopher Baines
  2021-04-22 21:02       ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-22 20:08 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
>> Small intentional changes are better, so I'd start just with looking
>> at some of the data coming out of the query. But yes, I think you're
>> in the right place. The hard part here is probably to look at how
>> those values are used in the JSON and HTML rendering code, and adjust
>> that accordingly.
>
> I made some modifications(moved the tests from controller.scm to
> comparison.scm) in order to remove the value "#f" from the outputs
> "hash-algorithm" and "hash" when rendering the html, for derivations
> that don't have these values set, but doing that now I'm showing them
> as empty fields on the json (instead of omitting them entirely).

I'm not quite sure what you mean by empty fields in the JSON here?

> This simplified a bit the processing on render-compare/derivations,
> and I think the html output is also better. For inputs, sources, etc,
> there are no processing being done on controller.scm other than the
> map. I can't see now what else could be done in this regard.
>
> An idea I had was to remove the entire map functions from the
> controller.scm to comparison.scm, but this would impact the html, so I
> would also have to modify the html, because the data format for json and
> html are slightly different. I'm not sure if this is what you want.

It sounds like you're roughly on the right track, do share what changes
you're making and then I can have a look.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-22 20:08     ` Christopher Baines
@ 2021-04-22 21:02       ` Luciana Lima Brito
  2021-04-22 21:15         ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-22 21:02 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

On Thu, 22 Apr 2021 21:08:08 +0100
Christopher Baines <mail@cbaines.net> wrote:

> I'm not quite sure what you mean by empty fields in the JSON here?

I mean now it appears like this in the json

hash-alg:	{}

Before, it was entirely omitted.
 
> It sounds like you're roughly on the right track, do share what
> changes you're making and then I can have a look.

This is the part I've changed on comparison.scm in the function
derivation-outputs-differences-data:

 (map (match-lambda
         ((output-name path hash-algorithm hash recursive
                       derivation_ids)
          (let ((parsed-derivation-ids
                 (map string->number
                      (parse-postgresql-array-string derivation_ids))))
            (list output-name
                  path
                  (if (string? hash-algorithm)
                          hash-algorithm
                          '())
                  (if (string? hash)
                      hash
                      '())
                  (string=? recursive "t")
                  (append (if (memq base-derivation-id
                                    parsed-derivation-ids)
                              '(base)
                              '())
                          (if (memq target-derivation-id
                                    parsed-derivation-ids)
                              '(target)
                              '()))))))
       (exec-query conn query))

Basically, I moved the hash-algorithm, hash, and recursive tests from
controller.scm here. I spent more time familiarizing with this part of
the code and the dataset. I don't have anything ready for the idea I
told you in the last email because I'm not so sure if that is the way
to go, but I can make some experiments in this regard.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-22 21:02       ` Luciana Lima Brito
@ 2021-04-22 21:15         ` Christopher Baines
  2021-04-23 21:15           ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-22 21:15 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Thu, 22 Apr 2021 21:08:08 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> I'm not quite sure what you mean by empty fields in the JSON here?
>
> I mean now it appears like this in the json
>
> hash-alg:	{}
>
> Before, it was entirely omitted.

Right, OK. I'd call that an empty object.

>> It sounds like you're roughly on the right track, do share what
>> changes you're making and then I can have a look.
>
> This is the part I've changed on comparison.scm in the function
> derivation-outputs-differences-data:
>
>  (map (match-lambda
>          ((output-name path hash-algorithm hash recursive
>                        derivation_ids)
>           (let ((parsed-derivation-ids
>                  (map string->number
>                       (parse-postgresql-array-string derivation_ids))))
>             (list output-name
>                   path
>                   (if (string? hash-algorithm)
>                           hash-algorithm
>                           '())
>                   (if (string? hash)
>                       hash
>                       '())
>                   (string=? recursive "t")
>                   (append (if (memq base-derivation-id
>                                     parsed-derivation-ids)
>                               '(base)
>                               '())
>                           (if (memq target-derivation-id
>                                     parsed-derivation-ids)
>                               '(target)
>                               '()))))))
>        (exec-query conn query))
>
> Basically, I moved the hash-algorithm, hash, and recursive tests from
> controller.scm here. I spent more time familiarizing with this part of
> the code and the dataset. I don't have anything ready for the idea I
> told you in the last email because I'm not so sure if that is the way
> to go, but I can make some experiments in this regard.

The bit for the recursive field looks fine.

I'd suggest avoiding '() as the value for hash and hash-algorithm when
they're NULL in the database. One option here that I've used in some
places is to return a alist rather than a list. This can simplify JSON
output since it's often a alist that's desired, and can avoid breaking
matches when they're matching on the list.

Does that make sense?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-22 21:15         ` Christopher Baines
@ 2021-04-23 21:15           ` Luciana Lima Brito
  2021-04-23 21:48             ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-23 21:15 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

Hi,

On Thu, 22 Apr 2021 22:15:34 +0100
Christopher Baines <mail@cbaines.net> wrote:

I'm stuck.
 
> I'd suggest avoiding '() as the value for hash and hash-algorithm when
> they're NULL in the database. One option here that I've used in some
> places is to return a alist rather than a list. This can simplify JSON
> output since it's often a alist that's desired, and can avoid breaking
> matches when they're matching on the list.
> 
> Does that make sense?

It is not so clear to me. I tried three ways, always testing on
"outputs":

First, I tried using an alist  within
the map function on derivation-outputs-differences-data
(comparison.scm), and I applied a list->vector on the result. It
worked, but I do believe that this is not the way we want it to be.
This way the label comes as another field, and I got the result like
this:

code:

`((label . ,(cond
		((and (memq base-derivation-id
			parsed-derivation-ids)
              	      (memq target-derivation-id
			parsed-derivation-ids))
                   'common)
         	((memq base-derivation-id parsed-derivation-ids)
		   'base)
              	(else 'target)))
           (name . ,output-name)
           (path . ,path)
           ,@(if (string? hash-algorithm)
                 `((hash-algorithm . ,hash-algorithm))
                 '())
           ,@(if (string? hash)
                 `((hash . ,hash))
                 '())
           (recursive . ,(string=? recursive "t")))

json outputs:
   0:
	label: "base"
	name: "foo"
	path: "bar"
	recursive: #f

This way I only used the function derivation-outputs-differences-data,
not using group-to-alist or group-by-last-element.

The second way I tried was to pass an alist as first element of the list
and append the labels (base or/and target):

(list
         `((name . ,output-name)
           (path . ,path)
           ,(if (string? hash-algorithm)
               `(hash-algorithm . ,hash-algorithm)
               '())
           ,(if (string? hash)
               `(hash . ,hash)
               '())
           (recursive . ,(string=? recursive "t")))
         
         (append (if (memq base-derivation-id
                           parsed-derivation-ids)
                     '(base)
                     '())
                 (if (memq target-derivation-id
                           parsed-derivation-ids)
                     '(target)
                     '())))

But this seemed to result in an extra parenthesis in the result, which
didn't work.

So I tried a third way, using the pairs
individually in the list and the append.

(list
         `(name . ,output-name)
         `(path . ,path)
         (if (string? hash-algorithm)
                 `(hash-algorithm . ,hash-algorithm)
                 '())
         (if (string? hash)
                 `(hash . ,hash)
                 '())
         `(recursive . ,(string=? recursive "t"))
         
         (append (if (memq base-derivation-id
                           parsed-derivation-ids)
                     '(base)
                     '())
                 (if (memq target-derivation-id
                           parsed-derivation-ids)
                     '(target)
                     '())))


Furthermore, all these methods would require tweaking the html
processing.

Here is an idea I just had. The first way I tried returned a
vector. This worked but kind of ugly. As I am already
getting the alist, I just thought about writing another function to
group by the first elements (the labels), something like this:

(group-by-first-element l)

where l is the alist I get from the first method I mentioned.
And then, I would have the following alist:
((base (...))
 (target (...))
 (common ((...)(...)(...))))

which is the current alist we are using in controller.scm.
Problem is, I still don't know how to do this, and this seems somewhat
too long winded to get the proper alist.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-23 21:15           ` Luciana Lima Brito
@ 2021-04-23 21:48             ` Christopher Baines
  2021-04-25 20:15               ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-23 21:48 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 5387 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
> On Thu, 22 Apr 2021 22:15:34 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> I'm stuck.
>
>> I'd suggest avoiding '() as the value for hash and hash-algorithm when
>> they're NULL in the database. One option here that I've used in some
>> places is to return a alist rather than a list. This can simplify JSON
>> output since it's often a alist that's desired, and can avoid breaking
>> matches when they're matching on the list.
>>
>> Does that make sense?
>
> It is not so clear to me. I tried three ways, always testing on
> "outputs":
>
> First, I tried using an alist  within
> the map function on derivation-outputs-differences-data
> (comparison.scm), and I applied a list->vector on the result. It
> worked, but I do believe that this is not the way we want it to be.
> This way the label comes as another field, and I got the result like
> this:
>
> code:
>
> `((label . ,(cond
> 		((and (memq base-derivation-id
> 			parsed-derivation-ids)
>               	      (memq target-derivation-id
> 			parsed-derivation-ids))
>                    'common)
>          	((memq base-derivation-id parsed-derivation-ids)
> 		   'base)
>               	(else 'target)))
>            (name . ,output-name)
>            (path . ,path)
>            ,@(if (string? hash-algorithm)
>                  `((hash-algorithm . ,hash-algorithm))
>                  '())
>            ,@(if (string? hash)
>                  `((hash . ,hash))
>                  '())
>            (recursive . ,(string=? recursive "t")))
>
> json outputs:
>    0:
> 	label: "base"
> 	name: "foo"
> 	path: "bar"
> 	recursive: #f
>
> This way I only used the function derivation-outputs-differences-data,
> not using group-to-alist or group-by-last-element.
>
> The second way I tried was to pass an alist as first element of the list
> and append the labels (base or/and target):
>
> (list
>          `((name . ,output-name)
>            (path . ,path)
>            ,(if (string? hash-algorithm)
>                `(hash-algorithm . ,hash-algorithm)
>                '())
>            ,(if (string? hash)
>                `(hash . ,hash)
>                '())
>            (recursive . ,(string=? recursive "t")))
>
>          (append (if (memq base-derivation-id
>                            parsed-derivation-ids)
>                      '(base)
>                      '())
>                  (if (memq target-derivation-id
>                            parsed-derivation-ids)
>                      '(target)
>                      '())))
>
> But this seemed to result in an extra parenthesis in the result, which
> didn't work.

I'd expect ,@(if ... rather than just ,(if ... and then having a list of
a pair when you want the if to result in something. There's some
documentation on quasiquoting (the ` thing) here:

  https://www.gnu.org/software/guile/manual/html_node/Expression-Syntax.html#index-quasiquote

> So I tried a third way, using the pairs
> individually in the list and the append.
>
> (list
>          `(name . ,output-name)
>          `(path . ,path)
>          (if (string? hash-algorithm)
>                  `(hash-algorithm . ,hash-algorithm)
>                  '())
>          (if (string? hash)
>                  `(hash . ,hash)
>                  '())
>          `(recursive . ,(string=? recursive "t"))
>
>          (append (if (memq base-derivation-id
>                            parsed-derivation-ids)
>                      '(base)
>                      '())
>                  (if (memq target-derivation-id
>                            parsed-derivation-ids)
>                      '(target)
>                      '())))
>
>
> Furthermore, all these methods would require tweaking the html
> processing.
>
> Here is an idea I just had. The first way I tried returned a
> vector. This worked but kind of ugly. As I am already
> getting the alist, I just thought about writing another function to
> group by the first elements (the labels), something like this:
>
> (group-by-first-element l)
>
> where l is the alist I get from the first method I mentioned.
> And then, I would have the following alist:
> ((base (...))
>  (target (...))
>  (common ((...)(...)(...))))
>
> which is the current alist we are using in controller.scm.
> Problem is, I still don't know how to do this, and this seems somewhat
> too long winded to get the proper alist.

So, the current code copes with the general case where there can be any
number of outputs, and some can be common between two derivations, and
some other number can come from just the base or target derivation.

The query will effectively return a list with an element for each
output.

I'd perhaps try to work within the current structure at first at least.

If you change derivation-outputs-differences-data to return a list of
alists rather than a list of lists, I'd look at adapting the
group-to-alist call to work with that.

The group-by-last-element procedure it passes in takes a list, and
returns a pair to add to the alist.

It seems like you need something like group-by-last-element, but
group-by... something in a alist, which just like group-by-last-element,
will take an alist, and return a pair, where the first element is the
thing to use as key, and the second element of the pair is the alist
tweaked to remove the key.

Does that make sense?

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-23 21:48             ` Christopher Baines
@ 2021-04-25 20:15               ` Luciana Lima Brito
  2021-04-26  8:15                 ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-25 20:15 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

Hi

Your advices helped me think more clearly.

There was no need to create or modify structures other than what I was
already changing. I now return an alist instead of a list on the
derivation-differences-* functions on comparison.scm (for outputs,
inputs and sources). It helped to simplify the mapping on
controller.scm. The changes on html.scm were minimal, basically it is
matching on pairs, instead of single values.

Two questions:

1 - The match on the html expects 5 values for "outputs", so I had to
settle on using empty objects on the JSON, when needed, else it would
break the match on the html. Is it ok?

2 - Now on controller.scm "outputs", "inputs", "sources", and even
"arguments" have the same structure, which is an alist of the form:

  ((base . (...))
   (target . (...))
   (common . (...)))

and I'm using the same map and match-lambda code to process them,
wouldn't it be reasonable now to make it a local function?

I'm sending the patch. I'll be waiting your reviews.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Change-handling-of-queried-data-for-derivations-comp.patch --]
[-- Type: text/x-patch, Size: 11946 bytes --]

From d605d519a684b1be57ebd09cdf697bcdba017da1 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 25 Apr 2021 15:17:33 -0300
Subject: [PATCH] Change handling of queried data for derivations comparison.

comparison.scm: return query data for derivation comparison as an alist, instead of list.
html.scm: match on pairs, instead of single values.
controller.scm: simplify mapping on outputs/inputs/sources.
---
 guix-data-service/comparison.scm             | 68 +++++++++++---------
 guix-data-service/web/compare/controller.scm | 62 ++++--------------
 guix-data-service/web/compare/html.scm       | 43 ++++++-------
 3 files changed, 69 insertions(+), 104 deletions(-)

diff --git a/guix-data-service/comparison.scm b/guix-data-service/comparison.scm
index e5e1955..1f47c38 100644
--- a/guix-data-service/comparison.scm
+++ b/guix-data-service/comparison.scm
@@ -158,19 +158,23 @@ GROUP BY 1, 2, 3, 4, 5"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list output-name
-                  path
-                  hash-algorithm
-                  hash
-                  recursive
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((output-name . ,output-name)
+              (path . ,path)
+              ,@(if (string? hash-algorithm)
+                    `((hash-algorithm . ,hash-algorithm))
+                    `((hash-algorithm . ())))
+              ,@(if (string? hash)
+                    `((hash . ,hash))
+                    `((hash . ())))
+              (recursive . ,(string=? recursive "t"))
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-inputs-differences-data conn
@@ -202,16 +206,16 @@ INNER JOIN derivations ON derivation_outputs.derivation_id = derivations.id
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-          (list derivation_file_name
-                derivation_output_name
-                (append (if (memq base-derivation-id
-                                  parsed-derivation-ids)
-                            '(base)
-                            '())
-                        (if (memq target-derivation-id
-                                  parsed-derivation-ids)
-                            '(target)
-                            '()))))))
+            `((derivation_file_name . ,derivation_file_name)
+              (derivation_output_name . ,derivation_output_name)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-sources-differences-data conn
@@ -235,15 +239,15 @@ GROUP BY derivation_source_files.store_path"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list store_path
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((store_path . ,store_path)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define* (package-derivation-differences-data conn
diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 895bb40..9ef8e5b 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -590,60 +590,24 @@
             ((application/json)
              (let ((outputs
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((name path hash-alg hash recursive)
-                                  `((name . ,name)
-                                    (path . ,path)
-                                    ,@(if (string? hash-alg)
-                                          `((hash-algorithm . ,hash-alg))
-                                          '())
-                                    ,@(if (string? hash)
-                                          `((hash . ,hash))
-                                          '())
-                                    (recursive . ,(string=? recursive "t")))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((output-groups (assq-ref data 'outputs)))
-                       (list (assq-ref output-groups 'base)
-                             (assq-ref output-groups 'target)
-                             (assq-ref output-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'outputs)))
 
                    (inputs
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation output)
-                                  `((derivation . ,derivation)
-                                    (output . ,output))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((input-groups (assq-ref data 'inputs)))
-                       (list (assq-ref input-groups 'base)
-                             (assq-ref input-groups 'target)
-                             (assq-ref input-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'inputs)))
 
                    (sources
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation)
-                                  `((derivation . ,derivation))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((source-groups (assq-ref data 'sources)))
-                       (list (assq-ref source-groups 'base)
-                             (assq-ref source-groups 'target)
-                             (assq-ref source-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'sources)))
 
                    (arguments
                     (map
@@ -651,7 +615,7 @@
                        ((label args ...)
                         `(,label . ,(list->vector args))))
                      (assq-ref data 'arguments))))
-
+               
                (render-json
                 `((base                  . ((derivation . ,base-derivation)))
                   (target                . ((derivation . ,target-derivation)))
diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..30cc499 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -487,27 +487,24 @@
                  (th "Hash")
                  (th "Recursive")))
                (tbody
-                ,@(let ((base-outputs (assq-ref outputs 'base))
-                        (target-outputs (assq-ref outputs 'target))
-                        (common-outputs (assq-ref outputs 'common)))
-                    (append-map
-                     (lambda (label items)
-                       (map
-                        (match-lambda
-                          ((name path hash-algorithm hash recursive)
-                           `(tr
-                             (td ,label)
-                             (td ,name)
-                             (td (a (@ (href ,path))
-                                    ,(display-store-item path)))
-                             (td ,hash-algorithm)
-                             (td ,hash)
-                             (td ,recursive))))
-                        (or items '())))
-                     (list base target "Common")
-                     (list (assq-ref outputs 'base)
-                           (assq-ref outputs 'target)
-                           (assq-ref outputs 'common))))))))
+                ,@(append-map
+                   (lambda (label items)
+                     (map
+                      (match-lambda
+                        (((_ . name) (_ . path) (_ . hash-alg) (_ . hash) (_ . recursive))
+                         `(tr
+                           (td ,label)
+                           (td ,name)
+                           (td (a (@ (href ,path))
+                                  ,(display-store-item path)))
+                           (td ,hash-alg)
+                           (td ,hash)
+                           (td ,recursive))))
+                      (or items '())))
+                   (list base target "Common")
+                   (list (assq-ref outputs 'base)
+                         (assq-ref outputs 'target)
+                         (assq-ref outputs 'common)))))))
         (h2 "Inputs")
         ,@(let ((inputs (assq-ref data 'inputs)))
             `((table
@@ -522,7 +519,7 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((derivation outputs)
+                        (((_ . derivation) (_ . outputs))
                          `(tr
                            (td ,label)
                            (td (a (@ (href ,derivation))
@@ -546,7 +543,7 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((file)
+                        (((_ . file))
                          `(tr
                            (td ,label)
                            (td (a (@ (href ,file))
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-25 20:15               ` Luciana Lima Brito
@ 2021-04-26  8:15                 ` Christopher Baines
  2021-04-26 19:11                   ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-26  8:15 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1768 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Your advices helped me think more clearly.

Great :)

> There was no need to create or modify structures other than what I was
> already changing. I now return an alist instead of a list on the
> derivation-differences-* functions on comparison.scm (for outputs,
> inputs and sources). It helped to simplify the mapping on
> controller.scm. The changes on html.scm were minimal, basically it is
> matching on pairs, instead of single values.
>
> Two questions:
>
> 1 - The match on the html expects 5 values for "outputs", so I had to
> settle on using empty objects on the JSON, when needed, else it would
> break the match on the html. Is it ok?

So, one advantage of alists over lists is that the code is probably less
brittle when adding elements say, since code parsing the list will
probably break with a new element, but this is probably less likely to
happen with an alist.

However, this will happen with an alist if match is used to pick
elements out. I'd suggest using assq-ref or similar to pluck elements
out.

> 2 - Now on controller.scm "outputs", "inputs", "sources", and even
> "arguments" have the same structure, which is an alist of the form:
>
>   ((base . (...))
>    (target . (...))
>    (common . (...)))
>
> and I'm using the same map and match-lambda code to process them,
> wouldn't it be reasonable now to make it a local function?

I'd consider these options first probably:

 - Could the data coming from derivation-differences-data have vectors
   where appropriate already? The HTML code would probably need to be
   adjusted, but I think that's fine.

 - Could this be written in a form like:

   ,@(map (lambda (name)
           ...)
          '(outputs inputs sources arguments))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-26  8:15                 ` Christopher Baines
@ 2021-04-26 19:11                   ` Luciana Lima Brito
  2021-04-26 21:21                     ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-26 19:11 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]

On Mon, 26 Apr 2021 09:15:37 +0100
Christopher Baines <mail@cbaines.net> wrote:

> So, one advantage of alists over lists is that the code is probably
> less brittle when adding elements say, since code parsing the list
> will probably break with a new element, but this is probably less
> likely to happen with an alist.
> 
> However, this will happen with an alist if match is used to pick
> elements out. I'd suggest using assq-ref or similar to pluck elements
> out.

Ok, I changed that on the html.scm.
 
> I'd consider these options first probably:
> 
>  - Could the data coming from derivation-differences-data have vectors
>    where appropriate already? The HTML code would probably need to be
>    adjusted, but I think that's fine.

I tried this for days but with no success. Maybe the only way would be
to tweak group-to-alist, but it touches many places, and I didn't want
to mess with it.

>  - Could this be written in a form like:
> 
>    ,@(map (lambda (name)
>            ...)
>           '(outputs inputs sources arguments))

This only make sense to me inside render-json (because of the ,@), but I
think the code would be less clean and "arguments" would appear in a
different order. What I did was bind the result of a function similar
to this in the let. 

Well, this way made things much shorter. I'm sending a new patch for
you to review.


-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Change-handling-of-queried-data-for-derivations-comp.patch --]
[-- Type: text/x-patch, Size: 13740 bytes --]

From d0e221f5aef2892582bdcb73aceadf937b50e45f Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 25 Apr 2021 15:17:33 -0300
Subject: [PATCH] Change handling of queried data for derivations comparison.

comparison.scm: return query data for derivation comparison as an alist, instead of list.
html.scm: Access derivation differences data using assq-ref.
controller.scm: generalize map for outputs/inputs/sources/arguments.
---
 guix-data-service/comparison.scm             | 68 ++++++++--------
 guix-data-service/web/compare/controller.scm | 82 ++++----------------
 guix-data-service/web/compare/html.scm       | 53 ++++++-------
 3 files changed, 75 insertions(+), 128 deletions(-)

diff --git a/guix-data-service/comparison.scm b/guix-data-service/comparison.scm
index e5e1955..1f47c38 100644
--- a/guix-data-service/comparison.scm
+++ b/guix-data-service/comparison.scm
@@ -158,19 +158,23 @@ GROUP BY 1, 2, 3, 4, 5"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list output-name
-                  path
-                  hash-algorithm
-                  hash
-                  recursive
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((output-name . ,output-name)
+              (path . ,path)
+              ,@(if (string? hash-algorithm)
+                    `((hash-algorithm . ,hash-algorithm))
+                    `((hash-algorithm . ())))
+              ,@(if (string? hash)
+                    `((hash . ,hash))
+                    `((hash . ())))
+              (recursive . ,(string=? recursive "t"))
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-inputs-differences-data conn
@@ -202,16 +206,16 @@ INNER JOIN derivations ON derivation_outputs.derivation_id = derivations.id
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-          (list derivation_file_name
-                derivation_output_name
-                (append (if (memq base-derivation-id
-                                  parsed-derivation-ids)
-                            '(base)
-                            '())
-                        (if (memq target-derivation-id
-                                  parsed-derivation-ids)
-                            '(target)
-                            '()))))))
+            `((derivation_file_name . ,derivation_file_name)
+              (derivation_output_name . ,derivation_output_name)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-sources-differences-data conn
@@ -235,15 +239,15 @@ GROUP BY derivation_source_files.store_path"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list store_path
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((store_path . ,store_path)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define* (package-derivation-differences-data conn
diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 895bb40..287ae97 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,79 +588,25 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (let ((outputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((name path hash-alg hash recursive)
-                                  `((name . ,name)
-                                    (path . ,path)
-                                    ,@(if (string? hash-alg)
-                                          `((hash-algorithm . ,hash-alg))
-                                          '())
-                                    ,@(if (string? hash)
-                                          `((hash . ,hash))
-                                          '())
-                                    (recursive . ,(string=? recursive "t")))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((output-groups (assq-ref data 'outputs)))
-                       (list (assq-ref output-groups 'base)
-                             (assq-ref output-groups 'target)
-                             (assq-ref output-groups 'common)))))
-
-                   (inputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation output)
-                                  `((derivation . ,derivation)
-                                    (output . ,output))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((input-groups (assq-ref data 'inputs)))
-                       (list (assq-ref input-groups 'base)
-                             (assq-ref input-groups 'target)
-                             (assq-ref input-groups 'common)))))
-
-                   (sources
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation)
-                                  `((derivation . ,derivation))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((source-groups (assq-ref data 'sources)))
-                       (list (assq-ref source-groups 'base)
-                             (assq-ref source-groups 'target)
-                             (assq-ref source-groups 'common)))))
-
-                   (arguments
-                    (map
-                     (match-lambda
-                       ((label args ...)
-                        `(,label . ,(list->vector args))))
-                     (assq-ref data 'arguments))))
-
+             (let ((data-groups
+                    (map (lambda (name)
+                           (cons name
+                                 (map
+                                  (match-lambda
+                                    ((label args ...)
+                                     `(,label . ,(list->vector args))))
+                                  (assq-ref data name))))
+                         '(outputs inputs sources arguments))))
+               
                (render-json
                 `((base                  . ((derivation . ,base-derivation)))
                   (target                . ((derivation . ,target-derivation)))
-                  (outputs               . ,outputs)
-                  (inputs                . ,inputs)
-                  (sources               . ,sources)
+                  (outputs               . ,(assq-ref data-groups 'outputs))
+                  (inputs                . ,(assq-ref data-groups 'inputs))
+                  (sources               . ,(assq-ref data-groups 'sources))
                   (system                . ,(assq-ref data 'system))
                   (builder               . ,(assq-ref data 'builder))
-                  (arguments             . ,arguments)
+                  (arguments             . ,(assq-ref data-groups 'arguments))
                   (environment-variables . ,(assq-ref
                                              data 'environment-variables)))
                 #:extra-headers http-headers-for-unchanging-content)))
diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..22e2dfc 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -487,27 +487,24 @@
                  (th "Hash")
                  (th "Recursive")))
                (tbody
-                ,@(let ((base-outputs (assq-ref outputs 'base))
-                        (target-outputs (assq-ref outputs 'target))
-                        (common-outputs (assq-ref outputs 'common)))
-                    (append-map
-                     (lambda (label items)
-                       (map
-                        (match-lambda
-                          ((name path hash-algorithm hash recursive)
-                           `(tr
-                             (td ,label)
-                             (td ,name)
-                             (td (a (@ (href ,path))
-                                    ,(display-store-item path)))
-                             (td ,hash-algorithm)
-                             (td ,hash)
-                             (td ,recursive))))
-                        (or items '())))
-                     (list base target "Common")
-                     (list (assq-ref outputs 'base)
-                           (assq-ref outputs 'target)
-                           (assq-ref outputs 'common))))))))
+                ,@(append-map
+                   (lambda (label items)
+                     (map
+                      (match-lambda
+                        ((alist ...)
+                         `(tr
+                           (td ,label)
+                           (td ,(assq-ref alist 'output-name))
+                           (td (a (@ (href ,(assq-ref alist 'path)))
+                                  ,(display-store-item (assq-ref alist 'path))))
+                           (td ,(assq-ref alist 'hash-algorithm))
+                           (td ,(assq-ref alist 'hash))
+                           (td ,(assq-ref alist 'recursive)))))
+                      (or items '())))
+                   (list base target "Common")
+                   (list (assq-ref outputs 'base)
+                         (assq-ref outputs 'target)
+                         (assq-ref outputs 'common)))))))
         (h2 "Inputs")
         ,@(let ((inputs (assq-ref data 'inputs)))
             `((table
@@ -522,12 +519,12 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((derivation outputs)
+                        ((alist ...)
                          `(tr
                            (td ,label)
-                           (td (a (@ (href ,derivation))
-                                  ,(display-store-item derivation)))
-                           (td ,outputs))))
+                           (td (a (@ (href ,(assq-ref alist 'derivation_file_name)))
+                                  ,(display-store-item (assq-ref alist 'derivation_file_name))))
+                           (td ,(assq-ref alist 'derivation_output_name)))))
                       (or items '())))
                    (list base target)
                    (list (assq-ref inputs 'base)
@@ -546,11 +543,11 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((file)
+                        ((alist ...)
                          `(tr
                            (td ,label)
-                           (td (a (@ (href ,file))
-                                  ,(display-store-item file))))))
+                           (td (a (@ (href ,(assq-ref alist 'store_path)))
+                                  ,(display-store-item (assq-ref alist 'store_path)))))))
                       (or items '())))
                    (list base target "Common")
                    (list (assq-ref sources 'base)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-26 19:11                   ` Luciana Lima Brito
@ 2021-04-26 21:21                     ` Christopher Baines
  2021-04-27 13:10                       ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-26 21:21 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2062 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Mon, 26 Apr 2021 09:15:37 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> So, one advantage of alists over lists is that the code is probably
>> less brittle when adding elements say, since code parsing the list
>> will probably break with a new element, but this is probably less
>> likely to happen with an alist.
>>
>> However, this will happen with an alist if match is used to pick
>> elements out. I'd suggest using assq-ref or similar to pluck elements
>> out.
>
> Ok, I changed that on the html.scm.

Great :)

Rather than writing:

  (match-lambda
    ((alist ...)

I'd just use

  (lambda (alist)

as I think that's equivalent right?

>> I'd consider these options first probably:
>>
>>  - Could the data coming from derivation-differences-data have vectors
>>    where appropriate already? The HTML code would probably need to be
>>    adjusted, but I think that's fine.
>
> I tried this for days but with no success. Maybe the only way would be
> to tweak group-to-alist, but it touches many places, and I didn't want
> to mess with it.

Maybe add another procedure that combines group-to-alist but generates
an alist with vectors as the values? (group-to-alist/vector maybe).

>>  - Could this be written in a form like:
>>
>>    ,@(map (lambda (name)
>>            ...)
>>           '(outputs inputs sources arguments))
>
> This only make sense to me inside render-json (because of the ,@), but I
> think the code would be less clean and "arguments" would appear in a
> different order. What I did was bind the result of a function similar
> to this in the let.

I think using let is OK, but I think just unpacking data-groups as
you've called it directly in to the alist is fine (so ,@data-groups),
rather than picking out the elements. JSON objects are unordered, so the
ordering isn't something that really matters.

If you do go down this route though, I'd probably add a comment saying
what things are being added to the outer most alist, just to make the
code quicker to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-26 21:21                     ` Christopher Baines
@ 2021-04-27 13:10                       ` Luciana Lima Brito
  2021-04-27 18:23                         ` Christopher Baines
  2021-04-27 18:33                         ` Luciana Lima Brito
  0 siblings, 2 replies; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-27 13:10 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On Mon, 26 Apr 2021 22:21:50 +0100
Christopher Baines <mail@cbaines.net> wrote:

> 
> Rather than writing:
> 
>   (match-lambda
>     ((alist ...)
> 
> I'd just use
> 
>   (lambda (alist)
> 
> as I think that's equivalent right?

Right, I did this.

> >> I'd consider these options first probably:
> >>
> >>  - Could the data coming from derivation-differences-data have
> >> vectors where appropriate already? The HTML code would probably
> >> need to be adjusted, but I think that's fine.  
> >
> > I tried this for days but with no success. Maybe the only way would
> > be to tweak group-to-alist, but it touches many places, and I
> > didn't want to mess with it.  
> 
> Maybe add another procedure that combines group-to-alist but generates
> an alist with vectors as the values? (group-to-alist/vector maybe).

> I think using let is OK, but I think just unpacking data-groups as
> you've called it directly in to the alist is fine (so ,@data-groups),
> rather than picking out the elements. JSON objects are unordered, so
> the ordering isn't something that really matters.
> 
> If you do go down this route though, I'd probably add a comment saying
> what things are being added to the outer most alist, just to make the
> code quicker to read.

Well, I went down the second route, now I'm calling the ,@data-groups
and I added a comment explaining its use.
The main point here is, the code is working and it looks nice, but to
get the data with the vectors seems to be right too. I'm sending the
new patch for your review and I'll wait for your call, if you think I
should try the first route or not.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Change-handling-of-queried-data-for-derivations-comp.patch --]
[-- Type: text/x-patch, Size: 13694 bytes --]

From 03a70ac2e07f2eec35a9473d8930a9cbefa50b92 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 25 Apr 2021 15:17:33 -0300
Subject: [PATCH] Change handling of queried data for derivations comparison.

comparison.scm: return query data for derivation comparison as an alist, instead of list.
html.scm: Access derivation differences data using assq-ref.
controller.scm: generalize map for outputs/inputs/sources/arguments.
---
 guix-data-service/comparison.scm             | 68 +++++++++--------
 guix-data-service/web/compare/controller.scm | 78 +++-----------------
 guix-data-service/web/compare/html.scm       | 62 +++++++---------
 3 files changed, 75 insertions(+), 133 deletions(-)

diff --git a/guix-data-service/comparison.scm b/guix-data-service/comparison.scm
index e5e1955..1f47c38 100644
--- a/guix-data-service/comparison.scm
+++ b/guix-data-service/comparison.scm
@@ -158,19 +158,23 @@ GROUP BY 1, 2, 3, 4, 5"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list output-name
-                  path
-                  hash-algorithm
-                  hash
-                  recursive
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((output-name . ,output-name)
+              (path . ,path)
+              ,@(if (string? hash-algorithm)
+                    `((hash-algorithm . ,hash-algorithm))
+                    `((hash-algorithm . ())))
+              ,@(if (string? hash)
+                    `((hash . ,hash))
+                    `((hash . ())))
+              (recursive . ,(string=? recursive "t"))
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-inputs-differences-data conn
@@ -202,16 +206,16 @@ INNER JOIN derivations ON derivation_outputs.derivation_id = derivations.id
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-          (list derivation_file_name
-                derivation_output_name
-                (append (if (memq base-derivation-id
-                                  parsed-derivation-ids)
-                            '(base)
-                            '())
-                        (if (memq target-derivation-id
-                                  parsed-derivation-ids)
-                            '(target)
-                            '()))))))
+            `((derivation_file_name . ,derivation_file_name)
+              (derivation_output_name . ,derivation_output_name)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-sources-differences-data conn
@@ -235,15 +239,15 @@ GROUP BY derivation_source_files.store_path"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list store_path
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((store_path . ,store_path)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define* (package-derivation-differences-data conn
diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 895bb40..a48b7c5 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,79 +588,23 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (let ((outputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((name path hash-alg hash recursive)
-                                  `((name . ,name)
-                                    (path . ,path)
-                                    ,@(if (string? hash-alg)
-                                          `((hash-algorithm . ,hash-alg))
-                                          '())
-                                    ,@(if (string? hash)
-                                          `((hash . ,hash))
-                                          '())
-                                    (recursive . ,(string=? recursive "t")))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((output-groups (assq-ref data 'outputs)))
-                       (list (assq-ref output-groups 'base)
-                             (assq-ref output-groups 'target)
-                             (assq-ref output-groups 'common)))))
-
-                   (inputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation output)
-                                  `((derivation . ,derivation)
-                                    (output . ,output))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((input-groups (assq-ref data 'inputs)))
-                       (list (assq-ref input-groups 'base)
-                             (assq-ref input-groups 'target)
-                             (assq-ref input-groups 'common)))))
-
-                   (sources
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation)
-                                  `((derivation . ,derivation))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((source-groups (assq-ref data 'sources)))
-                       (list (assq-ref source-groups 'base)
-                             (assq-ref source-groups 'target)
-                             (assq-ref source-groups 'common)))))
-
-                   (arguments
-                    (map
-                     (match-lambda
-                       ((label args ...)
-                        `(,label . ,(list->vector args))))
-                     (assq-ref data 'arguments))))
+             (let ((data-groups
+                    (map (lambda (name)
+                           (cons name
+                                 (map
+                                  (match-lambda
+                                    ((label args ...)
+                                     `(,label . ,(list->vector args))))
+                                  (assq-ref data name))))
+                         '(outputs inputs sources arguments))))
 
+               ;data-groups returns four pairs/entries: outputs, inputs, sources and arguments.
                (render-json
                 `((base                  . ((derivation . ,base-derivation)))
                   (target                . ((derivation . ,target-derivation)))
-                  (outputs               . ,outputs)
-                  (inputs                . ,inputs)
-                  (sources               . ,sources)
+                  ,@data-groups
                   (system                . ,(assq-ref data 'system))
                   (builder               . ,(assq-ref data 'builder))
-                  (arguments             . ,arguments)
                   (environment-variables . ,(assq-ref
                                              data 'environment-variables)))
                 #:extra-headers http-headers-for-unchanging-content)))
diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..d144736 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -487,27 +487,23 @@
                  (th "Hash")
                  (th "Recursive")))
                (tbody
-                ,@(let ((base-outputs (assq-ref outputs 'base))
-                        (target-outputs (assq-ref outputs 'target))
-                        (common-outputs (assq-ref outputs 'common)))
-                    (append-map
-                     (lambda (label items)
-                       (map
-                        (match-lambda
-                          ((name path hash-algorithm hash recursive)
-                           `(tr
-                             (td ,label)
-                             (td ,name)
-                             (td (a (@ (href ,path))
-                                    ,(display-store-item path)))
-                             (td ,hash-algorithm)
-                             (td ,hash)
-                             (td ,recursive))))
-                        (or items '())))
-                     (list base target "Common")
-                     (list (assq-ref outputs 'base)
-                           (assq-ref outputs 'target)
-                           (assq-ref outputs 'common))))))))
+                ,@(append-map
+                   (lambda (label items)
+                     (map
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td ,(assq-ref alist 'output-name))
+                          (td (a (@ (href ,(assq-ref alist 'path)))
+                                 ,(display-store-item (assq-ref alist 'path))))
+                          (td ,(assq-ref alist 'hash-algorithm))
+                          (td ,(assq-ref alist 'hash))
+                          (td ,(assq-ref alist 'recursive))))
+                      (or items '())))
+                   (list base target "Common")
+                   (list (assq-ref outputs 'base)
+                         (assq-ref outputs 'target)
+                         (assq-ref outputs 'common)))))))
         (h2 "Inputs")
         ,@(let ((inputs (assq-ref data 'inputs)))
             `((table
@@ -521,13 +517,12 @@
                 ,@(append-map
                    (lambda (label items)
                      (map
-                      (match-lambda
-                        ((derivation outputs)
-                         `(tr
-                           (td ,label)
-                           (td (a (@ (href ,derivation))
-                                  ,(display-store-item derivation)))
-                           (td ,outputs))))
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td (a (@ (href ,(assq-ref alist 'derivation_file_name)))
+                                 ,(display-store-item (assq-ref alist 'derivation_file_name))))
+                          (td ,(assq-ref alist 'derivation_output_name))))
                       (or items '())))
                    (list base target)
                    (list (assq-ref inputs 'base)
@@ -545,12 +540,11 @@
                 ,@(append-map
                    (lambda (label items)
                      (map
-                      (match-lambda
-                        ((file)
-                         `(tr
-                           (td ,label)
-                           (td (a (@ (href ,file))
-                                  ,(display-store-item file))))))
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td (a (@ (href ,(assq-ref alist 'store_path)))
+                                 ,(display-store-item (assq-ref alist 'store_path))))))
                       (or items '())))
                    (list base target "Common")
                    (list (assq-ref sources 'base)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 13:10                       ` Luciana Lima Brito
@ 2021-04-27 18:23                         ` Christopher Baines
  2021-04-27 18:33                         ` Luciana Lima Brito
  1 sibling, 0 replies; 20+ messages in thread
From: Christopher Baines @ 2021-04-27 18:23 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2394 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Mon, 26 Apr 2021 22:21:50 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>>
>> Rather than writing:
>>
>>   (match-lambda
>>     ((alist ...)
>>
>> I'd just use
>>
>>   (lambda (alist)
>>
>> as I think that's equivalent right?
>
> Right, I did this.

Great. Unfortunately I missed a few things similar to this when I last
looked.

For this bit:

  (match-lambda
    ((label args ...)
     `(,label . ,(list->vector args))))

Given the thing being matched is a pair in an alist, I'd use (label
. args). I'd also perhaps use cons rather than `(, . ,) for creating the
pair.

>> >> I'd consider these options first probably:
>> >>
>> >>  - Could the data coming from derivation-differences-data have
>> >> vectors where appropriate already? The HTML code would probably
>> >> need to be adjusted, but I think that's fine.
>> >
>> > I tried this for days but with no success. Maybe the only way would
>> > be to tweak group-to-alist, but it touches many places, and I
>> > didn't want to mess with it.
>>
>> Maybe add another procedure that combines group-to-alist but generates
>> an alist with vectors as the values? (group-to-alist/vector maybe).
>
>> I think using let is OK, but I think just unpacking data-groups as
>> you've called it directly in to the alist is fine (so ,@data-groups),
>> rather than picking out the elements. JSON objects are unordered, so
>> the ordering isn't something that really matters.
>>
>> If you do go down this route though, I'd probably add a comment saying
>> what things are being added to the outer most alist, just to make the
>> code quicker to read.
>
> Well, I went down the second route, now I'm calling the ,@data-groups
> and I added a comment explaining its use.
> The main point here is, the code is working and it looks nice, but to
> get the data with the vectors seems to be right too. I'm sending the
> new patch for your review and I'll wait for your call, if you think I
> should try the first route or not.

This looks OK now.

The other thing I forgot to mention last time was using empty objects
(() in the code) for the hash and hash-algorithm for derivations where
those aren't applicable. I'd probably suggest using the symbol null, or
not including those fields in the object.

Hopefully that's all the bits to fix up, apologies for not mentioning
them last time.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 13:10                       ` Luciana Lima Brito
  2021-04-27 18:23                         ` Christopher Baines
@ 2021-04-27 18:33                         ` Luciana Lima Brito
  2021-04-27 18:42                           ` Christopher Baines
  1 sibling, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-27 18:33 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

On Tue, 27 Apr 2021 13:10:01 +0000
Luciana Lima Brito <lubrito@posteo.net> wrote:
 
> > Maybe add another procedure that combines group-to-alist but
> > generates an alist with vectors as the values?
> > (group-to-alist/vector maybe).  

I did it! :)
It was so much simpler. I created a function
group-to-alist/vector, based on the previous function, to which I added
the map I was already using on the controller.scm for data-groups.

(define (group-to-alist/vector process lst)
  (map
   (match-lambda
     ((label args ...)
      `(,label . ,(list->vector args))))
   (group-to-alist process lst)))

The only change needed on the html.scm is to use vector->list in the
items, like this

(map
 (lambda (alist)
   ...
 (or (vector->list items) '()))

Please, tell me what you think is the best, this way or the other that
I sent earlier with the patch?


-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 18:33                         ` Luciana Lima Brito
@ 2021-04-27 18:42                           ` Christopher Baines
  2021-04-27 19:53                             ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-27 18:42 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Tue, 27 Apr 2021 13:10:01 +0000
> Luciana Lima Brito <lubrito@posteo.net> wrote:
>
>> > Maybe add another procedure that combines group-to-alist but
>> > generates an alist with vectors as the values?
>> > (group-to-alist/vector maybe).
>
> I did it! :)
> It was so much simpler. I created a function
> group-to-alist/vector, based on the previous function, to which I added
> the map I was already using on the controller.scm for data-groups.
>
> (define (group-to-alist/vector process lst)
>   (map
>    (match-lambda
>      ((label args ...)
>       `(,label . ,(list->vector args))))
>    (group-to-alist process lst)))
>
> The only change needed on the html.scm is to use vector->list in the
> items, like this
>
> (map
>  (lambda (alist)
>    ...
>  (or (vector->list items) '()))
>
> Please, tell me what you think is the best, this way or the other that
> I sent earlier with the patch?

Great :)

I'd go with this approach, applying the comments I made about the
match-lambda bit above in the email I sent a few minutes ago.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 18:42                           ` Christopher Baines
@ 2021-04-27 19:53                             ` Luciana Lima Brito
  2021-04-27 20:29                               ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-27 19:53 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

On Tue, 27 Apr 2021 19:42:13 +0100
Christopher Baines <mail@cbaines.net> wrote:

> I'd go with this approach, applying the comments I made about the
> match-lambda bit above in the email I sent a few minutes ago.

I applied all you instructed me. See if it looks better now.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Change-handling-of-queried-data-for-derivations-comp.patch --]
[-- Type: text/x-patch, Size: 17796 bytes --]

From 93ef30e06d7e10fd3a140c6f2a729d40540b05a8 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 25 Apr 2021 15:17:33 -0300
Subject: [PATCH] Change handling of queried data for derivations comparison.

comparison.scm: return query data for derivation comparison as an alist, instead of list.
html.scm: Access derivation differences data using assq-ref.
controller.scm: remove mapping for outputs/inputs/sources.
utils.scm: add group-to-alist/vector function.
---
 guix-data-service/comparison.scm             | 81 +++++++++---------
 guix-data-service/model/utils.scm            |  8 ++
 guix-data-service/web/compare/controller.scm | 88 +++-----------------
 guix-data-service/web/compare/html.scm       | 74 ++++++++--------
 4 files changed, 97 insertions(+), 154 deletions(-)

diff --git a/guix-data-service/comparison.scm b/guix-data-service/comparison.scm
index e5e1955..46f5377 100644
--- a/guix-data-service/comparison.scm
+++ b/guix-data-service/comparison.scm
@@ -74,19 +74,20 @@
               'value))
 
   `((outputs
-     . ,(group-to-alist
+     . ,(group-to-alist/vector
          group-by-last-element
          (derivation-outputs-differences-data conn
                                               (first base-derivation)
                                               (first target-derivation))))
     (inputs
-     . ,(group-to-alist
+     . ,(group-to-alist/vector
          group-by-last-element
          (derivation-inputs-differences-data conn
                                              (first base-derivation)
                                              (first target-derivation))))
+
     (sources
-     . ,(group-to-alist
+     . ,(group-to-alist/vector
          group-by-last-element
          (derivation-sources-differences-data conn
                                               (first base-derivation)
@@ -107,9 +108,9 @@
                         (target . ,target-builder))))
               (arguments
                . ,(if (eq? base-args target-args)
-                      `((common . ,base-args))
-                      `((base . ,base-args)
-                        (target . ,target-args))))
+                      `((common . ,(list->vector base-args)))
+                      `((base . ,(list->vector base-args))
+                        (target . ,(list->vector target-args)))))
               (environment-variables
                . ,(map (lambda (key)
                          (let ((base-value (fetch-value base-env-vars key))
@@ -158,19 +159,23 @@ GROUP BY 1, 2, 3, 4, 5"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list output-name
-                  path
-                  hash-algorithm
-                  hash
-                  recursive
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((output-name . ,output-name)
+              (path . ,path)
+              ,@(if (string? hash-algorithm)
+                    `((hash-algorithm . ,hash-algorithm))
+                    `((hash-algorithm . null)))
+              ,@(if (string? hash)
+                    `((hash . ,hash))
+                    `((hash . null)))
+              (recursive . ,(string=? recursive "t"))
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-inputs-differences-data conn
@@ -202,16 +207,16 @@ INNER JOIN derivations ON derivation_outputs.derivation_id = derivations.id
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-          (list derivation_file_name
-                derivation_output_name
-                (append (if (memq base-derivation-id
-                                  parsed-derivation-ids)
-                            '(base)
-                            '())
-                        (if (memq target-derivation-id
-                                  parsed-derivation-ids)
-                            '(target)
-                            '()))))))
+            `((derivation_file_name . ,derivation_file_name)
+              (derivation_output_name . ,derivation_output_name)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-sources-differences-data conn
@@ -235,15 +240,15 @@ GROUP BY derivation_source_files.store_path"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list store_path
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((store_path . ,store_path)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define* (package-derivation-differences-data conn
diff --git a/guix-data-service/model/utils.scm b/guix-data-service/model/utils.scm
index 13947bd..b11cee5 100644
--- a/guix-data-service/model/utils.scm
+++ b/guix-data-service/model/utils.scm
@@ -33,6 +33,7 @@
             deduplicate-strings
             group-list-by-first-n-fields
             group-to-alist
+            group-to-alist/vector
             insert-missing-data-and-return-all-ids))
 
 (define NULL '())
@@ -114,6 +115,13 @@
         '()
         lst))
 
+(define (group-to-alist/vector process lst)
+  (map
+   (match-lambda
+     ((label . items)
+      (cons label (list->vector items))))
+   (group-to-alist process lst)))
+
 (define (table-schema conn table-name)
   (let ((results
          (exec-query
diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 895bb40..8445185 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,82 +588,18 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (let ((outputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((name path hash-alg hash recursive)
-                                  `((name . ,name)
-                                    (path . ,path)
-                                    ,@(if (string? hash-alg)
-                                          `((hash-algorithm . ,hash-alg))
-                                          '())
-                                    ,@(if (string? hash)
-                                          `((hash . ,hash))
-                                          '())
-                                    (recursive . ,(string=? recursive "t")))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((output-groups (assq-ref data 'outputs)))
-                       (list (assq-ref output-groups 'base)
-                             (assq-ref output-groups 'target)
-                             (assq-ref output-groups 'common)))))
-
-                   (inputs
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation output)
-                                  `((derivation . ,derivation)
-                                    (output . ,output))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((input-groups (assq-ref data 'inputs)))
-                       (list (assq-ref input-groups 'base)
-                             (assq-ref input-groups 'target)
-                             (assq-ref input-groups 'common)))))
-
-                   (sources
-                    (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation)
-                                  `((derivation . ,derivation))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((source-groups (assq-ref data 'sources)))
-                       (list (assq-ref source-groups 'base)
-                             (assq-ref source-groups 'target)
-                             (assq-ref source-groups 'common)))))
-
-                   (arguments
-                    (map
-                     (match-lambda
-                       ((label args ...)
-                        `(,label . ,(list->vector args))))
-                     (assq-ref data 'arguments))))
-
-               (render-json
-                `((base                  . ((derivation . ,base-derivation)))
-                  (target                . ((derivation . ,target-derivation)))
-                  (outputs               . ,outputs)
-                  (inputs                . ,inputs)
-                  (sources               . ,sources)
-                  (system                . ,(assq-ref data 'system))
-                  (builder               . ,(assq-ref data 'builder))
-                  (arguments             . ,arguments)
-                  (environment-variables . ,(assq-ref
-                                             data 'environment-variables)))
-                #:extra-headers http-headers-for-unchanging-content)))
+             (render-json
+              `((base                  . ((derivation . ,base-derivation)))
+                (target                . ((derivation . ,target-derivation)))
+                (outputs               . ,(assq-ref data 'outputs))
+                (inputs                . ,(assq-ref data 'inputs))
+                (sources               . ,(assq-ref data 'sources))
+                (system                . ,(assq-ref data 'system))
+                (builder               . ,(assq-ref data 'builder))
+                (arguments             . ,(assq-ref data 'arguments))
+                (environment-variables . ,(assq-ref
+                                           data 'environment-variables)))
+              #:extra-headers http-headers-for-unchanging-content))
             (else
              (render-html
               #:sxml (compare/derivation
diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..e1ff15c 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -487,27 +487,23 @@
                  (th "Hash")
                  (th "Recursive")))
                (tbody
-                ,@(let ((base-outputs (assq-ref outputs 'base))
-                        (target-outputs (assq-ref outputs 'target))
-                        (common-outputs (assq-ref outputs 'common)))
-                    (append-map
-                     (lambda (label items)
-                       (map
-                        (match-lambda
-                          ((name path hash-algorithm hash recursive)
-                           `(tr
-                             (td ,label)
-                             (td ,name)
-                             (td (a (@ (href ,path))
-                                    ,(display-store-item path)))
-                             (td ,hash-algorithm)
-                             (td ,hash)
-                             (td ,recursive))))
-                        (or items '())))
-                     (list base target "Common")
-                     (list (assq-ref outputs 'base)
-                           (assq-ref outputs 'target)
-                           (assq-ref outputs 'common))))))))
+                ,@(append-map
+                   (lambda (label items)
+                     (map
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td ,(assq-ref alist 'output-name))
+                          (td (a (@ (href ,(assq-ref alist 'path)))
+                                 ,(display-store-item (assq-ref alist 'path))))
+                          (td ,(assq-ref alist 'hash-algorithm))
+                          (td ,(assq-ref alist 'hash))
+                          (td ,(assq-ref alist 'recursive))))
+                      (or (and=> items vector->list) '())))
+                   (list base target "Common")
+                   (list (assq-ref outputs 'base)
+                         (assq-ref outputs 'target)
+                         (assq-ref outputs 'common)))))))
         (h2 "Inputs")
         ,@(let ((inputs (assq-ref data 'inputs)))
             `((table
@@ -521,14 +517,13 @@
                 ,@(append-map
                    (lambda (label items)
                      (map
-                      (match-lambda
-                        ((derivation outputs)
-                         `(tr
-                           (td ,label)
-                           (td (a (@ (href ,derivation))
-                                  ,(display-store-item derivation)))
-                           (td ,outputs))))
-                      (or items '())))
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td (a (@ (href ,(assq-ref alist 'derivation_file_name)))
+                                 ,(display-store-item (assq-ref alist 'derivation_file_name))))
+                          (td ,(assq-ref alist 'derivation_output_name))))
+                      (or (and=> items vector->list) '())))
                    (list base target)
                    (list (assq-ref inputs 'base)
                          (assq-ref inputs 'target)))))))
@@ -545,13 +540,12 @@
                 ,@(append-map
                    (lambda (label items)
                      (map
-                      (match-lambda
-                        ((file)
-                         `(tr
-                           (td ,label)
-                           (td (a (@ (href ,file))
-                                  ,(display-store-item file))))))
-                      (or items '())))
+                      (lambda (alist)
+                        `(tr
+                          (td ,label)
+                          (td (a (@ (href ,(assq-ref alist 'store_path)))
+                                 ,(display-store-item (assq-ref alist 'store_path))))))
+                      (or (and=> items vector->list) '())))
                    (list base target "Common")
                    (list (assq-ref sources 'base)
                          (assq-ref sources 'target)
@@ -615,8 +609,8 @@
                            (td (ol
                                 ,@(map (lambda (arg)
                                          `(li ,(display-possible-store-item arg)))
-                                       (or common-args
-                                           base-args)))))
+                                       (or (and=> common-args vector->list)
+                                           (vector->list base-args))))))
                           (tr
                            (td ,target)
                            (td ,(display-possible-store-item
@@ -625,8 +619,8 @@
                            (td (ol
                                 ,@(map (lambda (arg)
                                          `(li ,(display-possible-store-item arg)))
-                                       (or common-args
-                                           target-args))))))))))))
+                                       (or (and=> common-args vector->list)
+                                           (vector->list target-args)))))))))))))
         (h2 "Environment variables")
         ,(let ((environment-variables (assq-ref data 'environment-variables)))
            `(table
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 19:53                             ` Luciana Lima Brito
@ 2021-04-27 20:29                               ` Christopher Baines
  2021-04-27 22:35                                 ` Luciana Lima Brito
  0 siblings, 1 reply; 20+ messages in thread
From: Christopher Baines @ 2021-04-27 20:29 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Tue, 27 Apr 2021 19:42:13 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> I'd go with this approach, applying the comments I made about the
>> match-lambda bit above in the email I sent a few minutes ago.
>
> I applied all you instructed me. See if it looks better now.

Great :) I've tweaked the commit message a little and pushed.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 20:29                               ` Christopher Baines
@ 2021-04-27 22:35                                 ` Luciana Lima Brito
  2021-04-28  7:56                                   ` Christopher Baines
  0 siblings, 1 reply; 20+ messages in thread
From: Luciana Lima Brito @ 2021-04-27 22:35 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

Hi,

On Tue, 27 Apr 2021 21:29:35 +0100
Christopher Baines <mail@cbaines.net> wrote:


> Great :) I've tweaked the commit message a little and pushed.

This is really great!!!

I started to look after my final application and I need to see two
specific things with you to finish filling the forms.

1 - There is a field in which I should provide more information, if
required by the community or project's mentor. Are there any questions
I should answer?

2 - About the timeline: "work with your mentor to provide a timeline of
the work you plan to accomplish on the project and what tasks you will
finish at each step".

another thing, what I should be doing now for my
next contribution?

I'll be waiting for your directions. 

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
  2021-04-27 22:35                                 ` Luciana Lima Brito
@ 2021-04-28  7:56                                   ` Christopher Baines
  0 siblings, 0 replies; 20+ messages in thread
From: Christopher Baines @ 2021-04-28  7:56 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
> On Tue, 27 Apr 2021 21:29:35 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>
>> Great :) I've tweaked the commit message a little and pushed.
>
> This is really great!!!
>
> I started to look after my final application and I need to see two
> specific things with you to finish filling the forms.
>
> 1 - There is a field in which I should provide more information, if
> required by the community or project's mentor. Are there any questions
> I should answer?

Nope, you can leave that bit.

> 2 - About the timeline: "work with your mentor to provide a timeline of
> the work you plan to accomplish on the project and what tasks you will
> finish at each step".

This is relevant though. I'd say the important bit of what's above is
the breakdown of the work (the tasks) as you see it, rather than any
exact dates in the timeline.

So, have another read of the project description and the main tasks as I
set out. Now is the time to discuss if that approach makes sense, and
once you're happy with the direction, start to split tasks down in to
more concrete parts.

I realise the performance improvements will be dependent on the
discovery work, so this more applies to the approach to work out what
bits are slow. I can try and help with this, but it's not me who's going
to be doing the work, so it's you who needs to be happy with the
proposed approach.

> another thing, what I should be doing now for my next contribution?

Given the final application deadline is quite close, I'd look at that
first.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-04-28  7:57 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 18:29 Outreachy - Guix Data Service: questions about improving the data for derivation comparisons Luciana Lima Brito
2021-04-22  7:53 ` Christopher Baines
2021-04-22 20:00   ` Luciana Lima Brito
2021-04-22 20:08     ` Christopher Baines
2021-04-22 21:02       ` Luciana Lima Brito
2021-04-22 21:15         ` Christopher Baines
2021-04-23 21:15           ` Luciana Lima Brito
2021-04-23 21:48             ` Christopher Baines
2021-04-25 20:15               ` Luciana Lima Brito
2021-04-26  8:15                 ` Christopher Baines
2021-04-26 19:11                   ` Luciana Lima Brito
2021-04-26 21:21                     ` Christopher Baines
2021-04-27 13:10                       ` Luciana Lima Brito
2021-04-27 18:23                         ` Christopher Baines
2021-04-27 18:33                         ` Luciana Lima Brito
2021-04-27 18:42                           ` Christopher Baines
2021-04-27 19:53                             ` Luciana Lima Brito
2021-04-27 20:29                               ` Christopher Baines
2021-04-27 22:35                                 ` Luciana Lima Brito
2021-04-28  7:56                                   ` Christopher Baines

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).