Adding type annotations and enabling auto-scoping #10173

jdunkerley · 2024-06-04T14:39:51Z

Pull Request Description

Renamed Missing_Required_Argument to Missing_Argument, and added throw method.
Add default widget to Case_Sensitivity.Insensitive locale.
Switch to auto scoping for parse_type_selector.
Add type annotation to various simple typed arguments in Table and DB_Table.
Altered Filter_Condition to have Missing_Argument for all non-defaulted arguments.
Added resolution of Column_Ref passed as auto-scoped to Table_Ref.
Altered Simple_Calculation to have Missing_Argument for all non-defaulted arguments.
Altered Join_Condition to have Missing_Argument for all non-defaulted arguments.
Altered Sort_Column to have Missing_Argument for all non-defaulted arguments.
Altered Aggregate_Column to have Missing_Argument for all non-defaulted arguments.

rename_columns:

aggregate:

join:

set:

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.

GregoryTravis · 2024-06-06T17:34:44Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Filter_Condition.enso

@@ -22,25 +23,25 @@ polyglot java import org.enso.base.Regex_Utils

 type Filter_Condition
    ## Is less than a value (or another column, in case of Table operations)?
-    Less than:Any action:Filter_Action=Filter_Action.Keep
+    Less than:Any=(Missing_Argument.throw "than") action:Filter_Action=Filter_Action.Keep


Why use throw over ensure_present?

didn't want the stack location etc - just the bare minimum error for now.

The source location was not being displayed anyway.

IMHO ensure_present has the nice advantage that it shows which function this argument belongs to (unless that's broken?). Then it makes it easier to localize where the missing thing is in a big expression (e.g. a join with many parameters).

But OK to just keep simple for now if you think that's better, we can always change in the future.

AdRiley · 2024-06-07T08:18:01Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Text/Case_Sensitivity.enso

@@ -21,6 +21,7 @@ type Case_Sensitivity

       Arguments:
       - locale: The locale used for the comparison.
+    @locale Locale.default_widget


Is this a defect somewhere else that you need to add this?

radeusgd · 2024-06-07T10:33:41Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Filter_Condition.enso

-        predicate : Function -> handle_constructor_missing_arguments predicate predicate
+        predicate : Function -> predicate
        element -> (== element)

-## PRIVATE
-   Checks if the given `function` is actually a `Filter_Condition` constructor
-   that is just missing arguments. If so, it will report a more friendly error.
-   Otherwise it will run the `continuation`.
-handle_constructor_missing_arguments function ~continuation =


Good that we no longer need this :)

distribution/lib/Standard/Base/0.0.0-dev/src/Errors/Common.enso

distribution/lib/Standard/Database/0.0.0-dev/src/DB_Table.enso

distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Table_Helpers.enso

radeusgd

Looks great, just a few small comments.

The Missing_Argument trick is really really awesome, it finally makes it so that we don't get some weird internal errors for 'half done' nodes but instead we get a clear message about a missing argument. This is really a great pattern that I hope we will use.

JaroslavTulach · 2024-06-08T14:33:59Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Filter_Condition.enso

@@ -22,25 +24,25 @@ polyglot java import org.enso.base.Regex_Utils

 type Filter_Condition
    ## Is less than a value (or another column, in case of Table operations)?
-    Less than:Any action:Filter_Action=Filter_Action.Keep
+    Less than=(Missing_Argument.throw "than") action:Filter_Action=Filter_Action.Keep


Is this still needed after merge of Execute foreign function and check autoscoped constructor result #10187 ? The message is composed at

enso/distribution/lib/Standard/Base/0.0.0-dev/src/Errors/Common.enso

Line 79 in 4aa3d52

_ -> ". Try to apply " + (missing_args.join ", ") + " arguments"

feel free to reformulate the sentece anyway you want.

Will try this in the next PR and if it works switch to then.

Hello @jdunkerley, does the

Execute foreign function and check autoscoped constructor result #10187

approach work for you? Can it be used instead of `Missing_Argument.throw "than", please? CCing @GregoryTravis

I think this is the best solution now because it requires no changes to Numbers.enso type signatures. It adds a special case to the error class but that seems acceptable.

JaroslavTulach · 2024-06-08T16:28:18Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Filter_Condition.enso

+
+    ## PRIVATE
+       Resolves a possibly auto-scoped value to a concrete value.
+    resolve_auto_scoped : Any -> Any


Please use private keyword.

Can't in this case as used across different libraries but yes will do.

jdunkerley added the CI: No changelog needed Do not require a changelog entry for this PR. label Jun 4, 2024

jdunkerley force-pushed the wip/jd/autoscoping branch 3 times, most recently from 24ba0b9 to 90d67cc Compare June 6, 2024 11:17

jdunkerley marked this pull request as ready for review June 6, 2024 14:49

jdunkerley requested review from radeusgd, GregoryTravis, AdRiley and marthasharkey as code owners June 6, 2024 14:49

GregoryTravis approved these changes Jun 6, 2024

View reviewed changes

AdRiley reviewed Jun 7, 2024

View reviewed changes

AdRiley approved these changes Jun 7, 2024

View reviewed changes

radeusgd reviewed Jun 7, 2024

View reviewed changes

distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Table_Helpers.enso Outdated Show resolved Hide resolved

radeusgd approved these changes Jun 7, 2024

View reviewed changes

jdunkerley force-pushed the wip/jd/autoscoping branch from 266ea2d to 9278fbc Compare June 7, 2024 15:23

jdunkerley requested review from 4e6, JaroslavTulach, hubertp and Akirathan as code owners June 7, 2024 17:23

jdunkerley force-pushed the wip/jd/autoscoping branch from 8efd872 to 4342405 Compare June 7, 2024 23:08

JaroslavTulach reviewed Jun 8, 2024

View reviewed changes

jdunkerley added 7 commits June 9, 2024 17:19

Enable auto-scoping for select_columns's case_sensitivity.

3fc1925

Enable more autoscoping.

ba5cd27

Add type argument for parse_type_selector.

397086e

Add type argument for parse_type_selector.

0251163

Down to Table.filter.

2dab8ab

Update the Filter_Condition to have errors for missing arguments.

9d32c2f

Rename Missing_Required_Argument. to Missing_Argument.

568f9e5

jdunkerley added 15 commits June 9, 2024 17:19

Rename Missing_Required_Argument. to Missing_Argument.

2f1338b

Add auto-scoping to Table.set.

352be81

More auto-scoping work.

6096ec1

Update text_cleanse.

e98a123

Use auto-scoping for order_by.

795f56f

Use auto-scoping for aggregate and cross_tab.

dd6a2b8

Update DB_Table.

6f8b36f

PR comments and failing tests.

19b16cd

Use Column_Ref in filter_condition dropdowns.

98359bb

Workthrough getting Filter_Condition work.

982f4ab

Re-enable autoscoping for Table.filter.

5508f50

Repairing Table tests. Adding some autoscoped tests.

0a93ce7

One more fix.

b1733b4

Resolve some issues.

32c16a7

Java format and aggregate tests.

df74d11

jdunkerley force-pushed the wip/jd/autoscoping branch from 1e14a09 to df74d11 Compare June 9, 2024 16:19

4e6 requested a review from JaroslavTulach June 9, 2024 17:04

hubertp approved these changes Jun 10, 2024

View reviewed changes

jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label Jun 10, 2024

mergify bot merged commit d938c96 into develop Jun 10, 2024
37 checks passed

mergify bot deleted the wip/jd/autoscoping branch June 10, 2024 07:52

enso-bot bot mentioned this pull request Jul 10, 2024

Comparable.from doesn't return Comparable #10355

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding type annotations and enabling auto-scoping #10173

Adding type annotations and enabling auto-scoping #10173

jdunkerley commented Jun 4, 2024 •

edited

Loading

GregoryTravis Jun 6, 2024

jdunkerley Jun 6, 2024

radeusgd Jun 7, 2024

AdRiley Jun 7, 2024

radeusgd Jun 7, 2024

radeusgd left a comment

JaroslavTulach Jun 8, 2024 •

edited

Loading

jdunkerley Jun 9, 2024

JaroslavTulach Jul 9, 2024 •

edited

Loading

GregoryTravis Jul 9, 2024

JaroslavTulach Jun 8, 2024

jdunkerley Jun 9, 2024

Adding type annotations and enabling auto-scoping #10173

Adding type annotations and enabling auto-scoping #10173

Conversation

jdunkerley commented Jun 4, 2024 • edited Loading

Pull Request Description

Important Notes

Checklist

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

radeusgd left a comment

Choose a reason for hiding this comment

JaroslavTulach Jun 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach Jul 9, 2024 • edited Loading

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

jdunkerley commented Jun 4, 2024 •

edited

Loading

JaroslavTulach Jun 8, 2024 •

edited

Loading

JaroslavTulach Jul 9, 2024 •

edited

Loading