Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page

[Christopher Baines <mail@cbaines.net>]

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


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

> On Fri, 16 Apr 2021 16:47:10 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> >> On the get-derivation-data function, I wouldn't use the same
>> >> function to process the inputs, outputs and sources. The data for
>> >> each is different, so I would separate the code as well.
>> >
>> > I understand that, but the logic to map the values for these three
>> > bindings is the same, wouldn't it generate redundancies implementing
>> > the same logic separately?
>>
>> I'm unsure three bindings are you referring to, can you clairfy?
>
> The bindings I was talking about are "outputs", "inputs" and "sources",
> the only difference between them are the number of elements each one
> has, that is why I simply made a match clause for each one. If you think
> it is better I can separate them. Also, do you think it would be clearer
> if I move each one inside the json case branch?

Ok, I think the data in the inputs, outputs and sources will be quite
different, because they're different things, right?

Looking at this page as an example [1], the inputs are each a derivation
and a string saying which output of that derivation is being used as the
input. The outputs have a number of bits of information, the name, path,
hash algorithm, hash and whether it's recursive, and the sources are
just a single string, the file.

1: https://data.guix.gnu.org/compare/derivation?base_derivation=/gnu/store/5s9mi0k3mk6cmakyckj2fcp4qb75rn79-debootstrap-1.0.123.drv&target_derivation=/gnu/store/vqhfs0dmfm4q0lr1w6sdhrbc2pa2jrar-debootstrap-1.0.123.drv

If it's the same function being used to process the inputs, outputs and
sources, then it's harder to quickly tell from the code what's going on,
as you don't know which code inside get-derivation-data applies to the
inputs, outputs or sources. Matching the code up directly with the data
it should process will make this much more obvious.

>> >> To avoid having to call a procedure three times, on the base,
>> >> target and common items, I'd consider following the same pattern
>> >> in the HTML generating code, map over a list of the lists, so
>> >> something like:
>> >>
>> >>   (map (lambda (name data)
>> >>          (cons name
>> >>                (match data
>> >>                  ((name path hash-alg hash recursive)
>> >>                   ...))))
>> >>        '(base target common)
>> >>        (list (assq-ref outputs 'base)
>> >>              (assq-ref outputs 'target)
>> >>              (assq-ref outputs 'common)))
>> >>
>> >> Does that make sense?
>> >
>> > Actually I did it in a similar way before, but it resulted in a list
>> > with all the values for base, target and common together, in which
>> > I had to have another way to separate them and render on json, for
>> > example, I tried appending "base", "target" or "common" names to
>> > each list (similar to your function?), but them I had to convert
>> > this list to a vector.
>>
>> Getting a list with all of the values in individually was possibly due
>> to using append-map rather than map.
>>
>> > Calling the function for each separately gave me a cleaner
>> > output. Also, I think that sometimes you might have more than one
>> > output for base, target like it does for common, and I fail to see
>> > how your example function addresses this. In short, I couldn't see
>> > the benefit of this over calling the function three times. Is it for
>> > organizational purpose or am I simply wrong? This time I'm just not
>> > understanding.
>>
>> It's an organisational thing, code is generally more readable if the
>> scope for variables is tight and there's less indirection. Defining a
>> procedure is one form of indirection. It's often really helpful, but I
>> think it's unnecessary here.
>>
>> You're right though about the above example not handling data being a
>> list, I think that's a fixable problem though, rather than the (match
>> data ...) bit, I'd suggest using map with match-lambda, probably
>> wrapped with list->vector if you want a vector which will be
>> outputted as a JSON array.
>>
>> Does that make sense?
>
> I still don't quite get it. The function I had before was like this
> (using inputs as example):
>
> (append-map
>  (lambda (label items)
>    (map
>     (match-lambda
>       ((derivation output)
>        (...))
>       (or items '()))))
>  (list "base" "target" "common")
>  (list (assq-ref inputs 'base)
>        (assq-ref inputs 'target)
>        (assq-ref inputs 'common)))
>
> Which indeed I made based on the html code. However this outputs me
> something like:
>
> (("base" derivation output)
>  ("target" derivation output)
>  ("common" derivation output)
>  ...)
>
> where "..." are lots of different ("common" derivation output) lists.
>
> The only way I could think of showing this, was transforming it to a
> vector, which gives us the indexes from 0, and inside each one we
> would have the label showing where it came from. Is that the way you
> think it is better? (is this what your proposed function should
> accomplish?)
>
> I think the above output produces a bit less clean json,
> that is why I changed this function to the last one I sent you, so I
> don't need to pass any labels, because if I pass the base, target, and
> common lists separately the output is already correct and we don't need
> any vectors.

So, append-map expects the procedure it runs on the elements in the
lists its provided to return a list. Whereas map would give you a list
of lists in this case, append-map effectively runs append on that list
of lists, so you end up with a list.

While a flatter list is what you want when building an HTML table, I
think you were looking to get a JSON object separating the common, base
and target elements, right? If so, then map, rather than append-map
should be more useful to you here. Since above you're passing in two
lists of three things, if the procedure passed to map returns a pair
with a string in the first position, you'll end up producing the scheme
version of a JSON object (an alist).

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