Skip to content

Conversation

@bitmaybewise
Copy link
Contributor

@bitmaybewise bitmaybewise commented May 5, 2023

Proposal

The function compactArray recursively flattens multiple arrays, eliminating null values.

Example:

{
  result: std.compactArray([1, [2, 3], ['4', null], []])
}

Outputs:

{
  "result": [1, 2, 3, "4"]
}

@google-cla
Copy link

google-cla bot commented May 5, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@bitmaybewise bitmaybewise force-pushed the flattening-flat-arrays branch from a3d6e12 to 2aa9ce1 Compare May 5, 2023 13:38
@bitmaybewise bitmaybewise marked this pull request as ready for review May 5, 2023 14:19
@bitmaybewise bitmaybewise changed the title Flat arrays should stay as is std.flattenArrays: Flat arrays should stay as is May 5, 2023
@sparkprime
Copy link
Contributor

It should give an error in that case. It's supposed to take an array of arrays and return an array.

@bitmaybewise bitmaybewise force-pushed the flattening-flat-arrays branch from 2aa9ce1 to eb1f94d Compare May 7, 2023 11:22
@bitmaybewise
Copy link
Contributor Author

bitmaybewise commented May 7, 2023

It should give an error in that case. It's supposed to take an array of arrays and return an array.

@sparkprime I believe you're right. It would be misleading given the function name.

I want to propose a new function to the stdlib, compactArray. I believe it would be better to have a new function. WDYT?

@bitmaybewise bitmaybewise changed the title std.flattenArrays: Flat arrays should stay as is std.compactArray: recursively flatten arrays May 7, 2023
@bitmaybewise bitmaybewise changed the title std.compactArray: recursively flatten arrays (Proposal) std.compactArray: recursively flatten arrays May 10, 2023
@bitmaybewise bitmaybewise force-pushed the flattening-flat-arrays branch from eb1f94d to 9398f91 Compare May 23, 2023 14:50
@bitmaybewise bitmaybewise changed the title (Proposal) std.compactArray: recursively flatten arrays feat: std.compactArray concatenates an array of arrays into a single flattened array, removing null values. May 23, 2023
@sparkprime
Copy link
Contributor

I think you can write it more simply like this:

local old_std = std;
local std = old_std {
  flattenDeep(value):
    if std.type(value) == 'array' then
      [y for x in value for y in std.flattenDeep(x)]
    else if value == null then
      []
    else
      [value],
};

{ 
  simpleVal: std.flattenDeep(0),
  simpleArr: std.flattenDeep([1, 2, 3]),
  mix: std.flattenDeep([1, [2, 3]]),
  mixWithNulls: std.flattenDeep([null, 1, null, [null, 2, null, 3, null]]),
  deeper: std.flattenDeep([[null], [[1], null], [null, 2, [null, [3, null]], null]]),
}

But actually I wonder why exclude null by default? If you remove the part that checks for null and returns [] then it will leave the nulls in the linear array. That might be useful in some cases. If you don't want them, you can remove them with [x in std.flattenDeep(foo) if x != null].

@sparkprime
Copy link
Contributor

BTW it wasn't clear to me if you wanted to support > 2 levels of nesting or whether the way you were recursively calling fold from within the accumulator would actually achieve that.

@bitmaybewise
Copy link
Contributor Author

local old_std = std;
local std = old_std {
  flattenDeep(value):
    if std.type(value) == 'array' then
      [y for x in value for y in std.flattenDeep(x)]
    else if value == null then
      []
    else
      [value],
};

{ 
  simpleVal: std.flattenDeep(0),
  simpleArr: std.flattenDeep([1, 2, 3]),
  mix: std.flattenDeep([1, [2, 3]]),
  mixWithNulls: std.flattenDeep([null, 1, null, [null, 2, null, 3, null]]),
  deeper: std.flattenDeep([[null], [[1], null], [null, 2, [null, [3, null]], null]]),
}

@sparkprime practically speaking, that is basically my proposal, but your implementation has the added benefit that if a single value is fed, it turns it into an array of a single value. It could be useful in some cases where the input could be either null or array(s).

I also like the approach you mentioned to strip nulls: [x in std.flattenDeep(foo) if x != null]. It is an extra step compared to the compactArray proposal, but simple and easy too. Array comprehensions tend to become difficult to read and understand, but in this case I don't believe it's harmful.

I usually prefer to represent the absence of values differently, and tend to strip nulls from third party libraries outputs, or transform them before feeding to other functions, but I guess it is a preference of mine rather than a good reason to not keep them.

By simplifying the function to not handle null values makes it simpler (as you said):

local old_std = std;
local std = old_std {
  flattenDeep(value):
    if std.type(value) == 'array' then
      [y for x in value for y in std.flattenDeep(x)]
    else
      [value],
{ 
  simpleVal: std.flattenDeep(0),
  simpleArr: std.flattenDeep([1, 2, 3]),
  mix: std.flattenDeep([1, [2, 3]]),
  mixWithNulls: std.flattenDeep([null, 1, null, [null, 2, null, 3, null]]),
  deeper: std.flattenDeep([[null], [[1], null], [null, 2, [null, [3, null]], null]]),
}

So thinking that not removing nulls would serve more purposes, I believe your suggestion will be more beneficial to the stdlib.

@bitmaybewise bitmaybewise force-pushed the flattening-flat-arrays branch 2 times, most recently from 1a5c906 to 0cd7338 Compare June 3, 2023 10:20
@bitmaybewise bitmaybewise changed the title feat: std.compactArray concatenates an array of arrays into a single flattened array, removing null values. feat: std.flattenDeepArray Concatenate an array containing values and arrays into a single flattened array. Jun 3, 2023
@bitmaybewise bitmaybewise force-pushed the flattening-flat-arrays branch from 0cd7338 to 7d8b015 Compare June 10, 2023 08:39
@sparkprime sparkprime merged commit fe8179a into google:master Jun 13, 2023
@sparkprime
Copy link
Contributor

In some ways [] already represents null in the sense that if you want to return an optional thing deep while building the structure, you can return [] instead of null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants