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.

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

Update Table API with new filter design #3750

Merged
merged 15 commits into from
Oct 5, 2022

Conversation

radeusgd
Copy link
Member

@radeusgd radeusgd commented Sep 29, 2022

Pull Request Description

Implements https://www.pivotaltracker.com/story/show/183389855

Important Notes

Implements basic filter operations both In-Memory and for the Database backend, ensuring that existing tests can be adapted and keep working. Not all Filter_Conditions are implemented yet.

Also implements significant part of https://www.pivotaltracker.com/story/show/183390314

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 ide build and ./run ide watch.

@radeusgd radeusgd force-pushed the wip/radeusgd/table-filter-in-memory-183389855 branch from f3c7c24 to 8d1cd87 Compare September 30, 2022 15:06
@radeusgd radeusgd self-assigned this Sep 30, 2022
@radeusgd radeusgd marked this pull request as ready for review September 30, 2022 18:46
@radeusgd radeusgd marked this pull request as draft October 3, 2022 08:55
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

Generally looks good - couple of minor things
And as discussed need to add the on_problem support (sorry for missing in design).

@radeusgd radeusgd force-pushed the wip/radeusgd/table-filter-in-memory-183389855 branch 2 times, most recently from 31c126f to 646e473 Compare October 4, 2022 22:58
@radeusgd radeusgd marked this pull request as ready for review October 4, 2022 23:23
@radeusgd radeusgd requested a review from jdunkerley October 5, 2022 07:46
@radeusgd radeusgd force-pushed the wip/radeusgd/table-filter-in-memory-183389855 branch from 1f610e4 to 485719a Compare October 5, 2022 07:47
@@ -854,7 +886,7 @@ type Table
table = Examples.inventory_table
double_inventory = table.at "total_stock" * 2
table.set "total_stock" double_inventory
set : Text -> Column.Column | Vector.Vector -> Table
set : Text -> Column | Vector.Vector -> Table
Copy link
Member

Choose a reason for hiding this comment

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

This change is wrong: Column is the module, Column.Column is the type.

But as am cleaning up a lot of this in next PR not worth blocking this merge over.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I think I was also trying to change the imports but then reverted that and this may be a leftover.

@radeusgd radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Oct 5, 2022
@mergify mergify bot merged commit 503d3eb into develop Oct 5, 2022
@mergify mergify bot deleted the wip/radeusgd/table-filter-in-memory-183389855 branch October 5, 2022 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants