Skip to content

Commit

Permalink
Convert AbstractString to String in set_attribute (#3840)
Browse files Browse the repository at this point in the history
  • Loading branch information
odow authored Oct 8, 2024
1 parent e78a3a4 commit 16b71e8
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/documentation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- uses: actions/checkout@v4
- uses: julia-actions/setup-julia@latest
with:
version: '1'
version: '1.10'
- name: Install dependencies
shell: julia --color=yes --project=docs/ {0}
run: |
Expand Down
30 changes: 26 additions & 4 deletions src/optimizer_interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,26 @@ function get_attribute(
return get_attribute(model, String(name))
end

# Some MOI attributes have a strict value type, like ::String or ::Float64, but
# users may pass other generic subtypes, like SubString instead of String.
# Rather than throw obtuse errors, we can catch and fix some common cases. We
# shouldn't fix every case (for example, AbstractString -> String) because
# users might intentionally want the other subtype.
#
# The default case is to do nothing:
_to_concrete_value_type_if_needed(::MOI.AnyAttribute, value) = value

# A common case is passing an AbstractString instead of a String.
function _to_concrete_value_type_if_needed(
attr::MOI.AnyAttribute,
value::AbstractString,
)
if !(value isa String) && MOI.attribute_value_type(attr) === String
return String(value)
end
return value
end

"""
set_attribute(model::GenericModel, attr::MOI.AbstractModelAttribute, value)
set_attribute(x::GenericVariableRef, attr::MOI.AbstractVariableAttribute, value)
Expand Down Expand Up @@ -1285,7 +1305,7 @@ function set_attribute(
attr::MOI.AbstractModelAttribute,
value,
)
MOI.set(model, attr, value)
MOI.set(model, attr, _to_concrete_value_type_if_needed(attr, value))
return
end

Expand All @@ -1294,7 +1314,8 @@ function set_attribute(
attr::MOI.AbstractVariableAttribute,
value,
)
MOI.set(owner_model(x), attr, x, value)
model = owner_model(x)
MOI.set(model, attr, x, _to_concrete_value_type_if_needed(attr, value))
return
end

Expand All @@ -1303,7 +1324,8 @@ function set_attribute(
attr::MOI.AbstractConstraintAttribute,
value,
)
MOI.set(owner_model(cr), attr, cr, value)
model = owner_model(cr)
MOI.set(model, attr, cr, _to_concrete_value_type_if_needed(attr, value))
return
end

Expand Down Expand Up @@ -1344,7 +1366,7 @@ function set_attribute(
attr::MOI.AbstractOptimizerAttribute,
value,
)
MOI.set(model, attr, value)
MOI.set(model, attr, _to_concrete_value_type_if_needed(attr, value))
return
end

Expand Down
25 changes: 25 additions & 0 deletions test/test_model.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1313,4 +1313,29 @@ function test_is_solved_and_feasible()
return
end

function test_set_abstract_string()
abstract_string = split("foo.bar", ".")[1]
model = Model() do
return MOI.Utilities.MockOptimizer(
MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()),
)
end
set_attribute(model, MOI.Name(), abstract_string)
@test get_attribute(model, MOI.Name()) == "foo"
@variable(model, x)
set_attribute(x, MOI.VariableName(), abstract_string)
ret = get_attribute(x, MOI.VariableName())
@test ret isa String && ret == "foo"
c = @constraint(model, 2x >= 1)
set_attribute(c, MOI.ConstraintName(), abstract_string)
ret = get_attribute(c, MOI.ConstraintName())
@test ret isa String && ret == "foo"
set_attribute(model, abstract_string, abstract_string)
ret = get_attribute(model, abstract_string)
# This `ret` is NOT a `::String` because we don't know the value type of the
# attribute.
@test ret isa typeof(abstract_string) && ret == "foo"
return
end

end # module TestModels

0 comments on commit 16b71e8

Please sign in to comment.