Skip to content

✨ Fix the union types #23

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 7, 2025
Merged

✨ Fix the union types #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions src/encoders/missingness_encoding/missingness_encoding.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ function missingness_encoder_fit(
Char => 'm',
),
)
supportedtypes = Union{Char, AbstractString, Number}
supportedtypes_list = [Char, AbstractString, Number]
supportedtypes = Union{supportedtypes_list...}

# 1. Define feature mapper
function feature_mapper(col, name)
Expand All @@ -50,7 +51,7 @@ function missingness_encoder_fit(

# Ensure label_for_missing keys are valid types
for possible_col_type in keys(label_for_missing)
if !(possible_col_type in union_types(supportedtypes))
if !(possible_col_type in supportedtypes_list)
throw(ArgumentError(VALID_TYPES_NEW_VAL_ME(possible_col_type)))
end
end
Expand All @@ -66,7 +67,7 @@ function missingness_encoder_fit(

# Get ancestor type of column
elgrandtype = nothing
for allowed_type in union_types(supportedtypes)
for allowed_type in supportedtypes_list
if col_type <: allowed_type
elgrandtype = allowed_type
break
Expand Down
7 changes: 4 additions & 3 deletions src/transformers/cardinality_reducer/cardinality_reducer.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ function cardinality_reducer_fit(
Char => 'O',
),
)
supportedtypes = Union{Char, AbstractString, Number}
supportedtypes_list = [Char, AbstractString, Number]
supportedtypes = Union{supportedtypes_list...}

# 1. Define feature mapper
function feature_mapper(col, name)
Expand All @@ -57,7 +58,7 @@ function cardinality_reducer_fit(

# Ensure label_for_infrequent keys are valid types
for possible_col_type in keys(label_for_infrequent)
if !(possible_col_type in union_types(supportedtypes))
if !(possible_col_type in supportedtypes_list)
throw(ArgumentError(VALID_TYPES_NEW_VAL(possible_col_type)))
end
end
Expand All @@ -71,7 +72,7 @@ function cardinality_reducer_fit(

# Get ancestor type of column
elgrandtype = nothing
for allowed_type in union_types(supportedtypes)
for allowed_type in supportedtypes_list
if col_type <: allowed_type
elgrandtype = allowed_type
break
Expand Down
4 changes: 1 addition & 3 deletions src/utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
# To go from e.g., Union{Integer, String} to (Integer, String)
union_types(x::Union) = (x.a, union_types(x.b)...)
union_types(x::Type) = (x,)
# add utility functions here
6 changes: 2 additions & 4 deletions test/transformers/cardinality_reducer.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using MLJTransforms: union_types, cardinality_reducer_fit, cardinality_reducer_transform
using MLJTransforms: cardinality_reducer_fit, cardinality_reducer_transform


@testset "Union_types" begin
@test union_types(Union{Integer, String}) == (Integer, String)
end

@testset "Throws errors when needed" begin
@test_throws ArgumentError begin
Expand Down
Loading