jq icon indicating copy to clipboard operation
jq copied to clipboard

Add builtin function `compact`

Open owenthereal opened this issue 2 years ago • 7 comments
trafficstars

compact removes nulls from an object.

owenthereal avatar Aug 12 '23 17:08 owenthereal

Implementation LGTM but i guess we have to decide if it goes into 1.7 or not? as i understood around the discussion about pick we wanted a compact but for arrays only but i could have misunderstood?

wader avatar Aug 14 '23 17:08 wader

You have to also include a regen of jq.1.prebuilt in order for the checks to pass.

nicowilliams avatar Aug 14 '23 18:08 nicowilliams

@wader:

Implementation LGTM but i guess we have to decide if it goes into 1.7 or not? as i understood around the discussion about pick we wanted a compact but for arrays only but i could have misunderstood?

I personally think that it's fine to go into 1.7 unless we think 1.7 should have zero new feature. compact could be the building blocks for other stuff as well besides improving the existing pick implementation.

@nicowilliams:

You have to also include a regen of jq.1.prebuilt in order for the checks to pass.

Fixed in https://github.com/jqlang/jq/pull/2838/commits/04a0dc5cf662afc5a88c6f88dbd1dcbacf70befc

This PR is ready for review again 😄

owenthereal avatar Aug 27 '23 00:08 owenthereal

@owenthereal wrote:

compact could be the building blocks for other stuff as well besides improving the existing pick implementation.

I am not sure whether the above refers to the behavior of pick on arrays or objects, but in either case, I would like to point out that the way pick inserts nulls in both cases is both intended and (at least in part) documented.

Consider in particular the fact that pick will add "null" values to object inputs, e.g. {} | pick(.x) produces {"x": null}.

This behavior fits in properly with the jq scheme of things, as evidenced for example by the way it flows from the simplicity of the def of pick. In this connection, it should also be remembered that, at least from the point of view of jq's .x selector, the absence of a key named "x" is equivalent to a null-valued key named "x". Thus pick(.x) can be understood as an imperative: evaluate .x and ensure the result is reflected in the output.

On the other hand, it might make sense for keep/1 (as being developed by @nicowilliams) to avoid inserting nulls into arrays and objects. Should {} | keep(.x) raise an exception?

pkoppstein avatar Aug 27 '23 03:08 pkoppstein

@wader wrote:

we have to decide if it goes into 1.7 or not?

My thinking is that it shouldn't, partly because we're already at jq 1.7rc2, but mainly because there hasn't been much discussion about it. Should we have guidelines as to when new trivial builtins should be added? In any case, it is very specific: in my experience, different users typically want to delete various combinations of distinguished values besides null, notably [], "", and/or other strings signifying the absence of a value.

pkoppstein avatar Aug 27 '23 04:08 pkoppstein

Personally, I think just del(.. | nulls) is easier to understand; do we want to merge this alias anyway?

emanuele6 avatar Dec 11 '23 10:12 emanuele6

Personally, I think just del(.. | nulls) is easier to understand; do we want to merge this alias anyway?

I tend to agree. I'm not sure I'd know what compact would do without first looking at the docs. I mean, that's often the case with jq builtins, but maybe we should try not to add more of these? The changes look ok besides that.

nicowilliams avatar Jan 16 '24 21:01 nicowilliams