Skip to content

Commit

Permalink
Small review refactorings
Browse files Browse the repository at this point in the history
  • Loading branch information
Akirathan committed Apr 14, 2023
1 parent 715fe94 commit 30109e1
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 25 deletions.
18 changes: 13 additions & 5 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Array.enso
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ type Array
Arguments:
- order: The order in which the array elements are sorted.
- on: A projection from the element type to the value of that element
being sorted on. If set to `Nothing` (the default argument),
being sorted on. If set to `Nothing` (the default),
identity function will be used.
- by: A function that compares the result of applying `on` to two
elements, returning an an `Ordering` if the two elements are comparable
Expand All @@ -183,6 +183,11 @@ type Array
- *Average Time:* `O(n * log n)`
- *Worst-Case Space:* `O(n)` additional

? Incomparable values
Incomparable values are either values with different comparators or with
the same comparator returning `Nothing` from its `compare` method.
See the documentation of the `Ordering` module for more info.

? Implementation Note
The sort implementation is based upon an adaptive, iterative mergesort
that requires far fewer than `n * log(n)` comparisons when the array
Expand All @@ -198,8 +203,11 @@ type Array
the comparators in the `self` array, with the exception of the group
for the default comparator, which is always the first group.

Additionally, a warning will be attached, explaining that incomparable
values were encountered.
Additionally, an `Incomparable_Values` dataflow error will be returned
if the `on_incomparable` parameter is set to `Problem_Behavior.Report_Error`,
or a warning attached if the `on_incomparable` parameter is set to
`Problem_Behavior.Report_Warning` in case of encountering incomparable
values.

It takes equal advantage of ascending and descending runs in the array,
making it much simpler to merge two or more sorted arrays: simply
Expand All @@ -216,8 +224,8 @@ type Array
[Pair 1 2, Pair -1 8].to_array.sort Sort_Direction.Descending (_.first)

> Example
Sorting an array with elements with different comparators. Values 1
and My_Type have different comparators. 1 will be sorted before My_Type
Sorting an array with elements with different comparators. Values `1`
and `My_Type` have different comparators. `1` will be sorted before `My_Type`
because it has the default comparator, and warning will be attached to
the resulting array.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ type Number

## Checks equality of numbers, using an `epsilon` value.

! Error Conditions
If either of the arguments is `Number.nan`, an `Incomparable_Values` error is raised.

Arguments:
- that: The number to check equality against.
- epsilon: The value by which `self` and `that` can be separated by before
Expand Down
26 changes: 15 additions & 11 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import project.Warning.Warning

import project.IO

# We have to import also conversion methods, therefore, we import all from the Ordering
# module
## We have to import also conversion methods, therefore, we import all from the Ordering
module
from project.Data.Ordering import all
from project.Data.Boolean import Boolean, True, False
from project.Data.Index_Sub_Range import Index_Sub_Range, take_helper, drop_helper
Expand Down Expand Up @@ -870,6 +870,11 @@ type Vector a
- *Average Time:* `O(n * log n)`
- *Worst-Case Space:* `O(n)` additional

? Incomparable values
Incomparable values are either values with different comparators or with
the same comparator returning `Nothing` from its `compare` method.
See the documentation of the `Ordering` module for more info.

? Implementation Note
The sort implementation is based upon an adaptive, iterative mergesort
that requires far fewer than `n * log(n)` comparisons when the vector
Expand All @@ -885,8 +890,11 @@ type Vector a
the comparators in the `self` vector, with the exception of the group
for the default comparator, which is always the first group.

Additionally, a warning will be attached, explaining that incomparable
values were encountered.
Additionally, an `Incomparable_Values` dataflow error will be returned
if the `on_incomparable` parameter is set to `Problem_Behavior.Report_Error`,
or a warning attached if the `on_incomparable` parameter is set to
`Problem_Behavior.Report_Warning` in case of encountering incomparable
values.

It takes equal advantage of ascending and descending runs in the array,
making it much simpler to merge two or more sorted arrays: simply
Expand All @@ -903,8 +911,8 @@ type Vector a
[Pair 1 2, Pair -1 8].sort Sort_Direction.Descending (_.first)

> Example
Sorting a vector with elements with different comparators. Values 1
and My_Type have different comparators. 1 will be sorted before My_Type
Sorting a vector with elements with different comparators. Values `1`
and `My_Type` have different comparators. `1` will be sorted before `My_Type`
because it has the default comparator, and warning will be attached to
the resulting vector.

Expand All @@ -915,11 +923,7 @@ type Vector a
True -> self.map it-> Comparable.from it
False -> self.map it-> Comparable.from (on it)
compare_funcs = comps.map (it-> it.compare)
problem_behavior = case on_incomparable of
Problem_Behavior.Ignore -> 0
Problem_Behavior.Report_Warning -> 1
Problem_Behavior.Report_Error -> 2
self.sort_builtin order.to_sign comps compare_funcs by on problem_behavior
self.sort_builtin order.to_sign comps compare_funcs by on on_incomparable.to_number

## UNSTABLE
Keeps only unique elements within the vector, removing any duplicates.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,11 @@ type Problem_Behavior
warnings = Warning.get_all result . map .value
cleared_result = Warning.set result []
self.attach_problems_after cleared_result warnings

## PRIVATE
Returns a mapping of Problem_Behavior constructors to an integer.
Used for sending the number to Java, rather than sending the atom.
to_number self = case self of
Ignore -> 0
Report_Warning -> 1
Report_Error -> 2
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.runtime.state.State;

/**
* Just a wrapper for {@code Array.sort_builtin} that delegates to {@code Vector.sort_builtin}. This
* is a temporary workaround until Array and Vector are fully merged into a single type.
*/
/** Just a wrapper for {@code Array.sort_builtin} that delegates to {@code Vector.sort_builtin}. */
@BuiltinMethod(type = "Array", name = "sort_builtin")
public final class SortArrayNode extends Node {
@Child private SortVectorNode sortVectorNode = SortVectorNode.build();
Expand Down
10 changes: 5 additions & 5 deletions test/Tests/src/Data/Ordering_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ type No_Comp_Type
expect_incomparable_warn : Any -> Any -> Any -> Nothing
expect_incomparable_warn left_val right_val result =
# Incomparable values warning wraps Text values in simple quotes
left_val_text = if Meta.is_a left_val Text then "'" + left_val + "'" else left_val.to_text
right_val_text = if Meta.is_a right_val Text then "'" + right_val + "'" else right_val.to_text
left_val_text = left_val.pretty
right_val_text = right_val.pretty
expected_warn_msg_left = "Values " + left_val_text + " and " + right_val_text + " are incomparable"
expected_warn_msg_right = "Values " + right_val_text + " and " + left_val_text + " are incomparable"
has_expected_warning = Warning.get_all result . map (_.value) . any (it-> it == expected_warn_msg_left || it == expected_warn_msg_right)
Expand All @@ -72,7 +72,7 @@ expect_no_warns result =

# === The Tests ===
spec =
topo_sort_pending = "Waiting for implementation of topological sort (https://github.com/enso-org/enso/issues/5742)"
topo_sort_pending = "Waiting for implementation of topological sort (https://github.com/enso-org/enso/issues/5834)"

Test.group "Default comparator" <|
Test.specify "should support custom comparator" <|
Expand Down Expand Up @@ -220,11 +220,11 @@ spec =
[My_Type.Value 1, Ord.Value 1, 20, 10].sort . should_equal [10, 20, My_Type.Value 1, Ord.Value 1]
[Ord.Value 1, 20, 10, My_Type.Value 1].sort . should_equal [10, 20, My_Type.Value 1, Ord.Value 1]

Test.specify "should be able to sort even unordered values" pending="https://github.com/enso-org/enso/issues/5834" <|
Test.specify "should be able to sort even unordered values" pending=topo_sort_pending <|
[Ord.Value 2, UPair.Value "a" "b", Ord.Value 1, UPair.Value "c" "d"].sort . should_equal [Ord.Value 2, Ord.Value 1, UPair.Value "a" "b", UPair.Value "c" "d"]
[Ord.Value 2, UPair.Value "X" "Y", Ord.Value 1, UPair.Value "c" "d"].sort . should_equal [Ord.Value 2, Ord.Value 1, UPair.Value "X" "Y", UPair.Value "c" "d"]

Test.specify "should produce warning when sorting unordered values" pending="https://github.com/enso-org/enso/issues/5834" <|
Test.specify "should produce warning when sorting unordered values" pending=topo_sort_pending <|
expect_incomparable_warn (UPair.Value 1 2) (UPair.Value 3 4) [UPair.Value 1 2, UPair.Value 3 4].sort


Expand Down

0 comments on commit 30109e1

Please sign in to comment.