feat(collection): add concat #748

chuang8511 · 2024-10-16T09:19:14Z

Because

we need to concat 2 arrays

This commit

add concat task in collection
add recipe samples

linear · 2024-10-16T09:19:17Z

INS-6334 Fix the wrong title of the JSON field

jvallesm

I added some suggestions for the documentation. Also some nitpicky comments, some things to consider (and perhaps address in a separate PR).

jvallesm · 2024-10-17T15:20:44Z

pkg/component/generic/collection/v0/.compogen/bottom.mdx

@@ -0,0 +1,64 @@
+


As mentioned in another PR, instead of adding newlines we should put that in the compogen template. This is a workaround and contributors (myself included) don't need to know about this.

I remove - in compogen when there is extra content

jvallesm · 2024-10-17T15:22:29Z

pkg/component/generic/collection/v0/.compogen/bottom.mdx

+
+## Example Recipes
+
+In this example, we are combining two arrays of strings using the `union` and `concat` components.


nit

Suggested change

In this example, we are combining two arrays of strings using the `union` and `concat` components.

### Combine 2 arrays using `TASK_UNION` and `TASK_CONCAT`

jvallesm · 2024-10-17T15:23:05Z

pkg/component/generic/collection/v0/.compogen/bottom.mdx

+```
+
+
+In this example, we are creating an object with keys `type` and `text` from a string.


nit

Suggested change

In this example, we are creating an object with keys `type` and `text` from a string.

### Use `TASK_ASSIGN` to create an object with `type` and `text` keys

jvallesm · 2024-10-17T15:26:15Z

pkg/component/generic/collection/v0/.compogen/bottom.mdx

+
+In this example, we are combining two arrays of strings using the `union` and `concat` components.
+```yaml
+variable:


How about documenting and providing examples with a comment here? E.g.

# This pipeline combines 2 arrays of strings # Examples: # ["foo", "bar"], ["zot", "bat"] -> ["foo", "bar", "zot", "bat"], []

I added these 2 in corresponding components.

# This component concatenates two arrays of strings. # Examples: # ["foo", "bar"], ["zot", "bat"] -> ["foo", "bar", "zot", "bat"]

# This component combines two arrays of strings in a union, ensuring that no duplicates are included. # Examples: # ["foo", "bar"], ["foo", "bat"] -> ["foo", "bar", "bat"]

I meant documented the input/output of the pipelines, not the individual components. But it's fine.

jvallesm · 2024-10-17T15:26:54Z

pkg/component/generic/collection/v0/config/tasks.json

@@ -316,5 +316,62 @@
      "title": "Output",
      "type": "object"
    }
+  },
+  "TASK_CONCAT": {
+    "instillShortDescription": "Concat the arrays. i.e. `[1, 2] + [3, 4] = [1, 2, 3, 4]`",


Suggested change

"instillShortDescription": "Concat the arrays. i.e. `[1, 2] + [3, 4] = [1, 2, 3, 4]`",

"instillShortDescription": "Concatenate the arrays. i.e. `[1, 2] + [3, 4] = [1, 2, 3, 4]`",

jvallesm · 2024-10-17T15:27:41Z

pkg/component/generic/collection/v0/config/tasks.json

+      "instillUIOrder": 0,
+      "properties": {
+        "arrays": {
+          "description": "Specify the arrays you want to concat.",


nit: this should be a description, not an order

Suggested change

"description": "Specify the arrays you want to concat.",

"description": "The arrays to be concatenated.",

🗣️ not crucial for this PR, just opening a discussion

We need to decide about the periods for the task tables. 2 options:

Always finish with a period

Don't finish with a period by default, only do so when we have > 1 sentence in the description (separated by a period).

cc @GeorgeWilliamStrong

Always finish with a period

Now, I follow this, which @GeorgeWilliamStrong mentioned it somewhere.

jvallesm · 2024-10-17T15:31:01Z

pkg/component/generic/collection/v0/config/tasks.json

+      "instillUIOrder": 0,
+      "properties": {
+        "array": {
+          "description": "The union set.",


This is a typo, right?

Suggested change

"description": "The union set.",

"description": "The concatenated arrays.",

jvallesm · 2024-10-17T15:32:04Z

pkg/component/generic/collection/v0/main.go

@@ -82,6 +85,19 @@ func (c *component) CreateExecution(x base.ComponentExecution) (base.IExecution,
 	return e, nil
 }

+func (e *execution) concat(in *structpb.Struct) (*structpb.Struct, error) {


nit: unit-testing this component would be great 🙏

chuang8511 · 2024-10-17T21:30:36Z

@jvallesm
Thanks for reviewing. I fixed all and made a few adjustments.
Please take a look.

And, I am curious about if there is a more elegant way to init the test data in your experience.

I felt it is not so easy to create the test data.

donch1989

It’s odd that compongen generates empty lines at the end of MDX files, but the pre-commit action removes them. I’ve adjusted the pre-commit hook to fix this behavior so the end-of-line characters in MDX files are no longer automatically removed. We can decide whether we should keep empty lines at the end of MDX files later.

jvallesm · 2024-10-18T05:38:03Z

It’s odd that compongen generates empty lines at the end of MDX files, but the pre-commit action removes them. I’ve adjusted the pre-commit hook to fix this behavior so the end-of-line characters in MDX files are no longer automatically removed. We can decide whether we should keep empty lines at the end of MDX files later.

It's a compromise between the readability of the template and the auto-generated file. Since the template ends with an optional section, this will produce a blank line by default (empty section). My opinion is that, while we should aim to make README.mdx as pretty as possible, the shape of autogenerated files isn't important as long as their rendered version (or e.g. the API of the package for auto-generated code) is neat. When in conflict we should prioritise rendered document > template > raw MDX.

jvallesm · 2024-10-18T05:44:54Z

And, I am curious about if there is a more elegant way to init the test data in your experience.

I felt it is not so easy to create the test data.

I can think of 2 simpler ways to define the test data:

As Go types ([]string{"c", "d"}, map[string]any{"array": want}). NewValue takes any type and will do the conversion for you.
You can also define the test data in JSON and use protojson to unmarshal it.

Because - we need to concat 2 arrays This commit - add concat task in collection - add recipe samples --------- Co-authored-by: Chang, Hui-Tang <[email protected]>

🤖 I have created a release *beep* *boop* --- ## [0.44.0-beta](v0.43.2-beta...v0.44.0-beta) (2024-10-22) ### Features * **collection:** add concat ([#748](#748)) ([04d1467](04d1467)) * **compogen:** improve Title Case capitalization ([#757](#757)) ([f956ead](f956ead)) * **component:** update documentation URL to component ID ([#749](#749)) ([d4083c2](d4083c2)) * **instillmodel:** implement instill model embedding ([#727](#727)) ([17d88bc](17d88bc)) * **run:** run logging data list by requester API ([#730](#730)) ([e1e844b](e1e844b)) * **slack:** enable OAuth 2.0 integration ([#758](#758)) ([8043dc5](8043dc5)) * standardize the tag naming convention ([#767](#767)) ([fd0500f](fd0500f)) * **web:** refactor web operator ([#753](#753)) ([700805f](700805f)) ### Bug Fixes * **groq:** use credential-supported model in example ([#752](#752)) ([fc90435](fc90435)) * **run:** not return minio error in list pipeline run ([#744](#744)) ([4d0afa1](4d0afa1)) * **whatsapp:** fix dir name ([#763](#763)) ([029aef9](029aef9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

chuang8511 added 2 commits October 16, 2024 10:03

feat(collection): add concat function

6c96711

chore(collection): add the sample for collection

30d0e81

chuang8511 requested review from GeorgeWilliamStrong, donch1989, pinglin, xiaofei-du and jvallesm as code owners October 16, 2024 09:19

droplet-bot added instill vdp instill component labels Oct 16, 2024

chore(collection): update doc

139c0f9

jvallesm reviewed Oct 17, 2024

View reviewed changes

chuang8511 added 4 commits October 17, 2024 21:59

chore(compogen): add blank line to template

d72fae0

chore(collection): add test and modify the doc

e299f03

chore(compogen): update blank line

233d20b

chore(component): update doc

da68466

chuang8511 requested a review from jvallesm October 17, 2024 21:28

chore: exclude mdx and svg files from end-of-file-fixer

32fcb81

donch1989 approved these changes Oct 18, 2024

View reviewed changes

donch1989 merged commit 04d1467 into main Oct 18, 2024
12 checks passed

donch1989 deleted the chunhao/ins-6334-concat branch October 18, 2024 04:14

droplet-bot mentioned this pull request Oct 18, 2024

chore(main): release 0.44.0-beta #747

Merged

joremysh pushed a commit that referenced this pull request Oct 18, 2024

feat(collection): add concat (#748)

acac04c

Because - we need to concat 2 arrays This commit - add concat task in collection - add recipe samples --------- Co-authored-by: Chang, Hui-Tang <[email protected]>

jvallesm mentioned this pull request Oct 21, 2024

feat(component,audio): add TASK_DETECT_ACTIVITY and TASK_SEGMENT #762

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(collection): add concat #748

feat(collection): add concat #748

chuang8511 commented Oct 16, 2024

linear bot commented Oct 16, 2024

jvallesm left a comment

jvallesm Oct 17, 2024

chuang8511 Oct 17, 2024

jvallesm Oct 17, 2024

jvallesm Oct 17, 2024

jvallesm Oct 17, 2024

chuang8511 Oct 17, 2024

jvallesm Oct 18, 2024

jvallesm Oct 17, 2024

jvallesm Oct 17, 2024

jvallesm Oct 17, 2024

chuang8511 Oct 17, 2024

jvallesm Oct 17, 2024

jvallesm Oct 17, 2024

chuang8511 commented Oct 17, 2024

donch1989 left a comment

jvallesm commented Oct 18, 2024

jvallesm commented Oct 18, 2024


		## Example Recipes

		In this example, we are combining two arrays of strings using the `union` and `concat` components.

	In this example, we are combining two arrays of strings using the `union` and `concat` components.
	### Combine 2 arrays using `TASK_UNION` and `TASK_CONCAT`

		```


		In this example, we are creating an object with keys `type` and `text` from a string.

	In this example, we are creating an object with keys `type` and `text` from a string.
	### Use `TASK_ASSIGN` to create an object with `type` and `text` keys

	"instillShortDescription": "Concat the arrays. i.e. `[1, 2] + [3, 4] = [1, 2, 3, 4]`",
	"instillShortDescription": "Concatenate the arrays. i.e. `[1, 2] + [3, 4] = [1, 2, 3, 4]`",

	"description": "Specify the arrays you want to concat.",
	"description": "The arrays to be concatenated.",

	"description": "The union set.",
	"description": "The concatenated arrays.",

feat(collection): add concat #748

feat(collection): add concat #748

Conversation

chuang8511 commented Oct 16, 2024

linear bot commented Oct 16, 2024

jvallesm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chuang8511 commented Oct 17, 2024

donch1989 left a comment

Choose a reason for hiding this comment

jvallesm commented Oct 18, 2024

jvallesm commented Oct 18, 2024