Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(array): add support to pipe into array.from #5165

Merged
merged 1 commit into from
Sep 9, 2022
Merged

feat(array): add support to pipe into array.from #5165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions a.flux
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import "array"
import "internal/debug"

array.from(rows: [{}])
|> debug.sink()
4 changes: 2 additions & 2 deletions libflux/go/libflux/buildinfo.gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ var sourceHashes = map[string]string{
"libflux/flux/templates/package.html": "635e1d638ad1a430f3894ff0a458572ca3c4dc6b9cec2c57fe771443849e1402",
"libflux/flux/templates/value.html": "2af699af2535f83635acbc75c92d2ee941c2335350fc7e82ceb2dccf479360bf",
"libflux/include/influxdata/flux.h": "a50070b13297a6427fc47488d4cfbf18d3c804a2c3cb6f7a6f918b0ff29715a8",
"stdlib/array/array.flux": "232cffbcddea8959e2de8a815d3e1d2b667450085c0cc6c27677a05ea4323cc6",
"stdlib/array/array.flux": "d0737bb4ee1cea99eb82d1fd6150f5686edd58d1c36a676593e623933dbcc424",
"stdlib/array/array_test.flux": "c4281ef1128ba6164c16f1dcb1b051d2231fe39617f23a070ae1942b5ddb70d5",
"stdlib/array/from_test.flux": "450810d4a5a8cc443462172fa7a0dee18f4a2c48bab1948a32a1c42935137f25",
"stdlib/array/from_test.flux": "688a3593d27aed7110d016c0b7634808dee533311d4349669a1104bc50a9f3e7",
"stdlib/bitwise/bitwise.flux": "580114d80074f27c12c28b5832e52269cd8f3a99890a53617c25aa778fd04acd",
"stdlib/bitwise/bitwise_test.flux": "150d847157570fe95f1ddeb65c75b7e30bb5fa8422df281d72b889b62e8db54c",
"stdlib/contrib/RohanSreerama5/naiveBayesClassifier/bayes_test.flux": "e02e637a7b50a0e1fb977491c349c547f9ab1d1daef4999735e73051c14291c1",
Expand Down
4 changes: 2 additions & 2 deletions stdlib/array/array.flux
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ package array
// records must have the same keys and data types.
//
// ## Parameters
// - rows: Array of records to construct a table with.
// - rows: Array of records to construct a table with. Default is the piped-forward array (`<-`).
//
// ## Examples
//
Expand Down Expand Up @@ -45,7 +45,7 @@ package array
//
// ## Metadata
// tags: inputs
builtin from : (rows: [A]) => stream[A] where A: Record
builtin from : (<-rows: [A]) => stream[A] where A: Record
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I'm the biggest fan of this. I can't really think of another example where we do this kind of thing. I saw the PR and thought I'd like it, but the test cases are a bit hard for me to read.

Is there a specific request asking for this or is this just a quality of life change? I'd like to get some opinions from others on the team if they feel it looks right before clicking approve. If others like it, I've got no problem with hitting approve from a functional standpoint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsternberg This is coming from me as I prepare content for the rollout of dynamic types. It just removes the need to assign an array to a variable to render it as a table when parsing JSON using the dynamic package:

import "array"
import "experimental/dynamic"

jsonData = bytes(v: "
    {
    \"items\":  [
        {\"name\": \"a\", \"pos\": {\"x\": 10, \"y\": 10, \"z\": 20}},
        {\"name\": \"b\", \"pos\": {\"x\": 30, \"y\": 99}},
        {\"name\": \"c\", \"pos\": {}},
        {\"name\": \"d\"}
        ]
    }
")

parsed = dynamic.parseJSON(data: jsonData)

parsed.items
    |> dynamic.asArray()
    |> array.map(
        fn: (x) => (
            {
                name: string(v: x.name),
                posX: int(v: x.pos.x),
                posY: int(v: x.pos.y)
            }
        )
    )
    |> array.from()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure. I'll just say I discourage people from using it this way outside of this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes with the json use case its a bit awkward when every other function has a pipeforward argument.

For example:

dynamic.parseJSON(...)
    |> dynamic.asArray()
    |> array.filter(fn: (x) => x.bar == 42)
    |> array.map(fn: (x) => ({foo: x.foo}))
    |> array.from()
    |> to()

Without this change users must do:

rows = dynamic.parseJSON(...)
    |> dynamic.asArray()
    |> array.filter(fn: (x) => x.bar == 42)
    |> array.map(fn: (x) => ({foo: x.foo}))
    
array.from(rows: rows)
    |> to()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @sanderson said. My github hadn't refreshed with the above comments before writing my reply.


// concat appends two arrays and returns a new array.
//
Expand Down
66 changes: 66 additions & 0 deletions stdlib/array/from_test.flux
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,69 @@ testcase from_group {

testing.diff(want, got) |> yield()
}
testcase from_pipe {
want =
csv.from(
csv:
"
#datatype,string,long,string,string,string,dateTime:RFC3339,boolean
#group,false,false,false,false,false,false,false
#default,_result,,,,,,
,result,table,_measurement,_field,t0,_time,_value
,,0,m0,f0,tagvalue,2018-12-19T22:13:30Z,false
,,0,m0,f0,tagvalue,2018-12-19T22:13:40Z,true
,,0,m0,f0,tagvalue,2018-12-19T22:13:50Z,false
,,0,m0,f0,tagvalue,2018-12-19T22:14:00Z,false
,,0,m0,f0,tagvalue,2018-12-19T22:14:10Z,true
,,0,m0,f0,tagvalue,2018-12-19T22:14:20Z,true
",
)
got =
[
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:13:30Z,
_value: false,
},
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:13:40Z,
_value: true,
},
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:13:50Z,
_value: false,
},
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:14:00Z,
_value: false,
},
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:14:10Z,
_value: true,
},
{
_measurement: "m0",
_field: "f0",
t0: "tagvalue",
_time: 2018-12-19T22:14:20Z,
_value: true,
},
]
|> array.from()

testing.diff(want, got) |> yield()
}