-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
Dict.pick and Dict.omit #2355
Comments
Here's the PR! #2365.⚡ Sweep Basic Tier: I'm creating this ticket using GPT-4. You have 3 GPT-4 tickets left for the month and 1 for the day. For more GPT-4 tickets, visit our payment portal.
Actions (click)
Install Sweep Configs: Pull Request
Step 1: 🔎 SearchingI found the following snippets in your repository. I will now analyze these snippets and come up with a plan. Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description.squiggle/packages/squiggle-lang/__tests__/library/list_test.ts Lines 1 to 68 in 13b14b8
squiggle/packages/squiggle-lang/src/fr/list.ts Lines 132 to 239 in 13b14b8
Lines 1 to 115 in 13b14b8
I also found the following external resources that might be helpful:Summaries of links found in the content: The page is a GitHub issue titled "Removal of _.omit in v5.0.0?" in the lodash/lodash repository. The issue was opened on January 12, 2017, and has 11 comments. The original poster noticed that _.omit has been removed in the master branch and is slated for removal in version 5.0.0. They couldn't find any discussion about the decision and are wondering why it was chosen to be removed. They mention that _.omit is one of their most frequently used Lodash methods and express uncertainty about how to achieve the same functionality with _.pick. Another contributor responds, explaining that _.omit is less optimal and more complex compared to _.pick or _.pickBy. They suggest being explicit in code and knowing the properties being consumed. They also mention using the Babel plugin transform-object-rest-spread as an alternative. The issue was closed as completed by another member. There are additional comments discussing the usage of _.omit and suggesting alternatives. Step 2: ⌨️ Coding
Modify packages/squiggle-lang/src/fr/list.ts with contents:
Run packages/squiggle-lang/src/fr/list.ts through the sandbox.
Modify packages/squiggle-lang/__tests__/library/list_test.ts with contents:
Run packages/squiggle-lang/__tests__/library/list_test.ts through the sandbox.
Create documentation_path with contents:
Modify packages/squiggle-lang/src/fr/list.ts with contents: Add the "pick" and "omit" functions to the list.ts file. The "pick" function should take a dictionary and an array of keys as arguments and return a new dictionary that only includes the key-value pairs where the key is included in the array of keys. The "omit" function should take a dictionary and an array of keys as arguments and return a new dictionary that excludes the key-value pairs where the key is included in the array of keys. Use the makeDefinition function to define these functions and add them to the maker object.
Run packages/squiggle-lang/src/fr/list.ts through the sandbox.
Modify packages/squiggle-lang/__tests__/library/list_test.ts with contents: Add tests for the "pick" and "omit" functions to the list_test.ts file. The tests should cover various scenarios including but not limited to: picking or omitting keys from an empty dictionary, picking or omitting keys that do not exist in the dictionary, and picking or omitting all keys from the dictionary. Use the testEvalToBe function to write these tests and add them to the describe block for "Dict functions".
Run packages/squiggle-lang/__tests__/library/list_test.ts through the sandbox.
Create changeset_path with contents:
Create packages/squiggle-lang/src/fr/dict.ts with contents: Create a new file "dict.ts" in the "packages/squiggle-lang/src/fr/" directory. This file will contain the "pick" and "omit" functions that were originally added to "list.ts". The "pick" function takes a dictionary and an array of keys and returns a new dictionary that only contains the key-value pairs for the specified keys. The "omit" function takes a dictionary and an array of keys and returns a new dictionary that does not contain the key-value pairs for the specified keys.
Create packages/squiggle-lang/__tests__/library/dict_test.ts with contents: Create a new file "dict_test.ts" in the "packages/squiggle-lang/__tests__/library/" directory. This file will contain the tests for the "pick" and "omit" functions that were originally added to "list_test.ts". The tests should cover various scenarios, including cases where the input dictionary is empty, the array of keys is empty, and the array of keys contains keys that are not in the input dictionary.
Run packages/squiggle-lang/__tests__/library/dict_test.ts through the sandbox.
Modify packages/squiggle-lang/src/fr/list.ts with contents: Remove the "pick" and "omit" functions from this file, as they have been moved to "dict.ts".
Run packages/squiggle-lang/src/fr/list.ts through the sandbox.
Modify packages/squiggle-lang/__tests__/library/list_test.ts with contents: Remove the tests for the "pick" and "omit" functions from this file, as they have been moved to "dict_test.ts".
Run packages/squiggle-lang/__tests__/library/list_test.ts through the sandbox.
Step 3: 🔁 Code ReviewI have finished reviewing the code for completeness. I did not find errors for 🎉 Latest improvements to Sweep:
💡 To recreate the pull request edit the issue title or description. To tweak the pull request, leave a comment on the pull request. |
Description of suggestion or shortcoming:
Lodash has an Object.pick() and Object.omit() methods. It seems useful and simple to bring these to Rescript.
One potential downside to note: _.omit() is being removed from Lodash 5.0, due to performance issues. My quick guess is that this shouldn't be as major of a deal in Rescript, but I could be wrong here.
lodash/lodash#2930
You'll have to edit fr/list.ts, library/list_test.ts in squiggle-language, and the list documentation page.
Also, make a changeset.
Checklist
packages/squiggle-lang/src/fr/list.ts
✗packages/squiggle-lang/__tests__/library/list_test.ts
✗documentation_path
✓ 05018a5documentation_path
✗packages/squiggle-lang/src/fr/list.ts
✗packages/squiggle-lang/__tests__/library/list_test.ts
✗documentation_path
✗changeset_path
✓ a4030ddchangeset_path
✗packages/squiggle-lang/src/fr/dict.ts
⋯packages/squiggle-lang/src/fr/dict.ts
✗packages/squiggle-lang/__tests__/library/dict_test.ts
⋯packages/squiggle-lang/src/fr/list.ts
▶packages/squiggle-lang/__tests__/library/list_test.ts
▶changeset_path
▶The text was updated successfully, but these errors were encountered: