Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Restructuring the Faker type and creating tests for Group_By #3318

Merged
merged 20 commits into from
Mar 9, 2022
Merged

Restructuring the Faker type and creating tests for Group_By #3318

merged 20 commits into from
Mar 9, 2022

Conversation

jdunkerley
Copy link
Member

@jdunkerley jdunkerley commented Mar 3, 2022 •
edited
Loading

Pull Request Description

  • Added Minimum, Maximum, Longest. Shortest, Mode, Percentile
  • Added first and last to Map
  • Restructured Faker type more inline with FakerJS
  • Created 2,500 row data set
  • Tests for group_by
  • Performance tests for group_by

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH ./run dist and ./run watch.

@jdunkerley jdunkerley force-pushed the wip/jd/group-by-tests-181414898 branch 4 times, most recently from c257ca5 to 47b4f2c Compare March 7, 2022 17:51
jdunkerley added 15 commits March 8, 2022 13:27
@jdunkerley
Faker restructure
4991c5b
@jdunkerley
Change Faker API
1aaebc5 
Aligns much more with fakerjs
@jdunkerley
Fix existing tests
b0728ce
@jdunkerley
Fix one more Context in Array.Copy
0a09e73 
Benchmark script for `group_by`
@jdunkerley
Working on full test suite for Group_By
df74f4e
@jdunkerley
Some tests...
83eb12f
@jdunkerley
Naming clashes and more tests
b4f25dd
@jdunkerley
Adding more tests
7abc4f6 
Adding Min, Max, Shortest, Longest
@jdunkerley
Final full table tests
c0bbee1 
Adding empty table tests
@jdunkerley
Grouping on empty table
b399426
@jdunkerley
Added Mode and Percentile
b612d22 
Added ability to ignore Nothing in Count Distinct
Added Map first and last utility methods
More tests for group by
@jdunkerley
Fix Group_By key to work better
cd2cd32 
Test for count distinct ignoring nothing
@jdunkerley
Fix Mode
0e68429
@jdunkerley
Percentile fixed
d81c458 
Added Mode and Percentile tests
@jdunkerley
First and Last test on Map
2402902 
Fixes for percentile
Remove Boolean special handling
@jdunkerley jdunkerley force-pushed the wip/jd/group-by-tests-181414898 branch from 25de2a0 to 2402902 Compare March 8, 2022 14:02
@jdunkerley jdunkerley marked this pull request as ready for review March 8, 2022 15:58
@jdunkerley jdunkerley requested review from 4e6 and radeusgd as code owners March 8, 2022 15:58
radeusgd
radeusgd approved these changes Mar 8, 2022
View reviewed changes
Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Some reservations to what we should return for first and last of an empty map.

@jdunkerley jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label Mar 9, 2022
@mergify mergify bot merged commit 65465fb into develop Mar 9, 2022
@mergify mergify bot deleted the wip/jd/group-by-tests-181414898 branch March 9, 2022 10:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@4e6 4e6 4e6 approved these changes

@radeusgd radeusgd radeusgd approved these changes

Assignees
No one assigned
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jdunkerley @4e6 @radeusgd