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

Use new Enso Hash Codes and Comparable #6060

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ type Comparable
hash_callback : Atom -> Integer
hash_callback atom = (Comparable.from atom).hash atom

## PRIVATE
A callback allowing to compare two atoms with a custom comparator.
compare_callback : Atom -> Atom -> Integer | Nothing
compare_callback atom that =
ordering = Ordering.compare atom that
if ordering.is_error then Nothing else ordering.to_sign

## PRIVATE
A custom comparator is any comparator that is different than the
default ones.
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import project.Data.Array.Array
import project.Data.Numbers.Decimal
import project.Data.Numbers.Integer
import project.Data.Numbers.Number
import project.Data.Ordering.Comparator
import project.Data.Ordering.Ordering
import project.Data.Ordering.Comparable
import project.Data.Range.Extensions
Expand Down Expand Up @@ -66,7 +65,7 @@ type Rank_Method
Error.throw (Incomparable_Values.Error exc.payload.getLeftOperand exc.payload.getRightOperand)

handle_cmp_exc <| handle_nullpointer <|
java_ranks = Rank.rank input.to_array Comparator.new java_method
java_ranks = Rank.rank input.to_array java_method
Vector.from_polyglot_array java_ranks

type Statistic
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import project.Data.Numbers.Integer

from project.Data.Ordering import all
from project.Data.Boolean import Boolean, True, False


polyglot java import java.time.DayOfWeek

type Day_Of_Week
Expand Down Expand Up @@ -34,8 +37,9 @@ type Day_Of_Week
Day_Of_Week.Friday -> 5
Day_Of_Week.Saturday -> 6

shifted = if first_day == Day_Of_Week.Sunday then day_number else
(day_number + 7 - (first_day.to_integer start_at_zero=True)) % 7
shifted = case first_day of
Day_Of_Week.Sunday -> day_number
_ -> (day_number + 7 - (first_day.to_integer start_at_zero=True)) % 7
Comment on lines +40 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
shifted = case first_day of
Day_Of_Week.Sunday -> day_number
_ -> (day_number + 7 - (first_day.to_integer start_at_zero=True)) % 7
shifted = (day_number + 7 - (first_day.to_integer start_at_zero=True)) % 7

btw. won't just that work? I'm not sure if we have to special-case the Sunday given that we do % later anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

stack overflow - to_integer would be called each time,

Copy link
Member

Choose a reason for hiding this comment

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

I believe the culprit is #6065.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right! Okay then let's keep it.

I guess it would have worked if we did:

        day_number day = case day of
            Day_Of_Week.Sunday -> 0
            Day_Of_Week.Monday -> 1
            Day_Of_Week.Tuesday -> 2
            Day_Of_Week.Wednesday -> 3
            Day_Of_Week.Thursday -> 4
            Day_Of_Week.Friday -> 5
            Day_Of_Week.Saturday -> 6
        shifted = ((day_number self) + 7 - (day_number first_day)) % 7

right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This was only changed to avoid having == in it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I was just wondering on a side if we could further simplify it, removing the possibility of the infinite loop altogether.


shifted + if start_at_zero then 0 else 1

Expand All @@ -49,3 +53,14 @@ type Day_Of_Week
Day_Of_Week.Thursday -> DayOfWeek.THURSDAY
Day_Of_Week.Friday -> DayOfWeek.FRIDAY
Day_Of_Week.Saturday -> DayOfWeek.SATURDAY

## PRIVATE
type Day_Of_Week_Comparator
Copy link
Member

Choose a reason for hiding this comment

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

This looks exactly as I have envisioned that when dreaming about Comparator design.

compare x y =
x_int = x.to_integer
y_int = y.to_integer
Comparable.from x_int . compare x_int y_int

hash x = x.to_integer

Comparable.from (_:Day_Of_Week) = Day_Of_Week_Comparator
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import project.Data.Text.Text
import project.Nothing.Nothing

from project.Data.Ordering import all
from project.Data.Boolean import Boolean, False, True

polyglot java import org.enso.base.Http_Utils
Expand All @@ -14,23 +16,6 @@ type Header
- value: The header value.
Value name value

## Header equality.

Arguments:
- that: The header to compare against.

> Example
Compare two headers.

import Standard.Base.Network.HTTP.Header.Header

example_header_eq =
(Header.new "My_Header" "foo") == (Header.new "My_Header" "bar")
== : Header -> Boolean
== self that = case that of
_ : Header -> (self.name.equals_ignore_case that.name) && self.value==that.value
_ -> False

## ALIAS Build a Header

Create a new Header.
Expand Down Expand Up @@ -182,3 +167,15 @@ type Header
example_header_text_plain = Header.text_plain
text_plain : Header
text_plain = Header.content_type "text/plain"

## PRIVATE
type Header_Comparator
compare x y =
if x.name.equals_ignore_case y.name && x.value == y.value then Ordering.Equal else
Nothing

hash x =
key = x.name.to_case_insensitive_key + x.value
Comparable.from key . hash key

Comparable.from (_:Header) = Header_Comparator
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from Standard.Base import all
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument

from Standard.Base.Data.Ordering import all

import project.Data.Column.Column

polyglot java import java.sql.Types
Expand All @@ -15,11 +17,6 @@ type SQL_Type
- name: a database-specific type name, used for pretty printing.
Value typeid name

== self that = case that of
SQL_Type.Value that_id _ ->
self.typeid == that_id
_ -> False

## The SQL representation of `Boolean` type.
boolean : SQL_Type
boolean = SQL_Type.Value Types.BOOLEAN "BOOLEAN"
Expand Down Expand Up @@ -187,3 +184,13 @@ merge_number_type left right = if left.is_definitely_integer then merge_number_t
if right_index.is_nothing then left else
new_index = left_index.max right_index
[SQL_Type.numeric, SQL_Type.decimal, SQL_Type.real, SQL_Type.real, SQL_Type.double].at new_index

## PRIVATE
type SQL_Type_Comparator
compare x y =
if x.typeid == y.typeid then Ordering.Equal else
Nothing

hash x = x.typeid.hashCode

Comparable.from (_:SQL_Type) = SQL_Type_Comparator
36 changes: 12 additions & 24 deletions distribution/lib/Standard/Image/0.0.0-dev/src/Data/Image.enso
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from Standard.Base import all
import Standard.Base.Errors.File_Error.File_Error

from Standard.Base.Data.Ordering import all

import project.Read_Flag.Read_Flag
import project.Write_Flag.Write_Flag
import project.Data.Histogram.Histogram
Expand Down Expand Up @@ -382,30 +384,6 @@ type Image
/ : (Number | Vector | Matrix) -> Image ! Matrix_Error
/ self value = Panic.recover Any (core_op self.opencv_mat value (Java_Image.divide _ _ _)) . catch Any core_op_handler

## UNSTABLE

Check the equality of two images.

Arguments:
- that: the matrix to compare with.

? Implementation Note
Two images considered equal when they have the same number of rows,
columns and channels, and have the same pixel values.

The image represented internally as a byte array, and if two images
have the same dimensions, equality checks that underlying byte arrays
are equal as well.

> Example
Checking two images for equality.

import Standard.Examples

example_eq = Examples.image == Examples.image
== : Image -> Boolean
== self that = Java_Image.is_equals self.opencv_mat that.opencv_mat

## UNSTABLE

Convert the image to a vector.
Expand Down Expand Up @@ -499,3 +477,13 @@ core_op_handler error =
case error of
Matrix_Error.Dimensions_Not_Equal -> Error.throw error
_ -> Panic.throw error

## PRIVATE
type Image_Comparator
compare x y =
if Java_Image.is_equals x.opencv_mat y.opencv_mat then Ordering.Equal else
Nothing

hash x = x.opencv_mat.hashCode
Copy link
Member

Choose a reason for hiding this comment

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

Good. Delegating to Java hashCode when one knows there is a Java object is good.


Comparable.from (_:Image) = Image_Comparator
32 changes: 12 additions & 20 deletions distribution/lib/Standard/Image/0.0.0-dev/src/Data/Matrix.enso
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from Standard.Base import all

from Standard.Base.Data.Ordering import all

import project.Data.Image.Image
import project.Data.Matrix_Error.Matrix_Error

Expand Down Expand Up @@ -334,26 +336,6 @@ type Matrix
/ : (Number | Vector | Matrix) -> Matrix ! Matrix_Error
/ self value = Panic.recover Any (core_op self.opencv_mat value (Java_Matrix.divide _ _ _)) . catch core_op_handler

## UNSTABLE

Check the equality of two matrices.

Arguments:
- that: the matrix to compare with.

? Implementation Note
Two matrices considered equal when they have the same number of rows,
columns and channels, and have the same values.

> Example
Comparing two matrices for equality.

import Standard.Examples

example_eq = Examples.matrix == Examples.matrix
== : Matrix -> Boolean
== self that = Java_Matrix.is_equals self.opencv_mat that.opencv_mat

## UNSTABLE

Normalize the matrix into a range of [min_value .. max_value] so that the
Expand Down Expand Up @@ -460,3 +442,13 @@ core_op_handler error =
case error of
Matrix_Error.Dimensions_Not_Equal -> Error.throw error
_ -> Panic.throw error

## PRIVATE
type Matrix_Comparator
compare x y =
if Java_Matrix.is_equals x.opencv_mat y.opencv_mat then Ordering.Equal else
Nothing

hash x = x.opencv_mat.hashCode

Comparable.from (_:Matrix) = Matrix_Comparator
27 changes: 17 additions & 10 deletions distribution/lib/Standard/Table/0.0.0-dev/src/Data/Column.enso
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from Standard.Base import all
import Standard.Base.Data.Array_Proxy.Array_Proxy
import Standard.Base.Data.Ordering.Comparator
import Standard.Base.Errors.Common.Index_Out_Of_Bounds
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
import Standard.Base.Errors.Illegal_State.Illegal_State
Expand Down Expand Up @@ -1258,15 +1257,23 @@ type Column
my_compare a b = Ordering.compare a.abs b.abs
Examples.decimal_column.sort by=my_compare
sort : Sort_Direction -> Boolean -> (Any -> Any -> Ordering) | Nothing -> Column
sort self order=Sort_Direction.Ascending missing_last=True by=Nothing =
order_bool = case order of
Sort_Direction.Ascending -> True
Sort_Direction.Descending -> False
java_cmp = Comparator.new by
rule = OrderBuilder.OrderRule.new self.java_column java_cmp order_bool missing_last
mask = OrderBuilder.buildOrderMask [rule].to_array
new_col = self.java_column.applyMask mask
Column.Value new_col
sort self order=Sort_Direction.Ascending missing_last=True by=Nothing = case by of
Nothing ->
order_bool = case order of
Sort_Direction.Ascending -> True
Sort_Direction.Descending -> False
rule = OrderBuilder.OrderRule.new self.java_column order_bool missing_last
mask = OrderBuilder.buildOrderMask [rule].to_array
new_col = self.java_column.applyMask mask
Column.Value new_col
_ ->
wrapped a b = case a of
Nothing -> if b.is_nothing then Ordering.Equal else if missing_last then Ordering.Greater else Ordering.Less
_ -> case b of
Nothing -> if missing_last then Ordering.Less else Ordering.Greater
_ -> by a b
Comment on lines +1270 to +1274
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it could be a separate method that will be useful in other places:
make_comparator_nullsafe missing_last by = a-> b-> ...

sorted = self.to_vector.sort order by=wrapped
Column.from_vector self.name sorted

## Creates a new Column with the specified range of rows from the input
Column.
Expand Down
18 changes: 12 additions & 6 deletions distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from Standard.Base import all
import Standard.Base.Data.Array_Proxy.Array_Proxy
import Standard.Base.Data.Index_Sub_Range as Index_Sub_Range_Module
import Standard.Base.Data.Ordering.Comparator
import Standard.Base.Errors.Common.Incomparable_Values
import Standard.Base.Errors.Common.Index_Out_Of_Bounds
import Standard.Base.Errors.Common.No_Such_Method
Expand Down Expand Up @@ -47,6 +46,7 @@ from project.Errors import all
from project.Data.Column import get_item_string, normalize_string_for_display
from project.Internal.Filter_Condition_Helpers import make_filter_column

polyglot java import org.enso.base.ObjectComparator
polyglot java import org.enso.table.data.column.builder.object.StorageTypeMismatch
polyglot java import org.enso.table.data.table.Table as Java_Table
polyglot java import org.enso.table.data.table.Column as Java_Column
Expand Down Expand Up @@ -684,7 +684,16 @@ type Table
c.column.java_column
directions = columns_for_ordering.map c->
c.associated_selector.direction.to_sign
comparator = Comparator.for_text_ordering text_ordering

comparator = case text_ordering.sort_digits_as_numbers of
True ->
txt_cmp a b = Natural_Order.compare a b text_ordering.case_sensitivity . to_sign
ObjectComparator.new txt_cmp
False -> case text_ordering.case_sensitivity of
Case_Sensitivity.Default -> ObjectComparator.DEFAULT
Case_Sensitivity.Sensitive -> ObjectComparator.DEFAULT
Case_Sensitivity.Insensitive locale -> ObjectComparator.new False locale.java_locale

java_table = Illegal_Argument.handle_java_exception <| Incomparable_Values.handle_errors <|
self.java_table.orderBy java_columns.to_array directions.to_array comparator
Table.Value java_table
Expand Down Expand Up @@ -1280,11 +1289,8 @@ type Table
join_resolution = make_join_helpers self right . resolve on on_problems
right_columns_to_drop = join_resolution.redundant_column_names

object_comparator = Comparator.new
equality_fallback = .==

java_conditions = join_resolution.conditions
new_java_table = self.java_table.join right.java_table java_conditions (rows_to_keep.at 0) (rows_to_keep.at 1) (rows_to_keep.at 2) (columns_to_keep.at 0) (columns_to_keep.at 1) right_columns_to_drop right_prefix object_comparator equality_fallback
new_java_table = self.java_table.join right.java_table java_conditions (rows_to_keep.at 0) (rows_to_keep.at 1) (rows_to_keep.at 2) (columns_to_keep.at 0) (columns_to_keep.at 1) right_columns_to_drop right_prefix

on_problems.attach_problems_after (Table.Value new_java_table) <|
problems = new_java_table.getProblems
Expand Down
Loading