Skip to content

Conversation

blegat
Copy link
Member

@blegat blegat commented May 10, 2021

Extracted from #1287

Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

Needs tests for SingleVariable and AbstractVectorFunction.

Can't they also be simplified to:

throw_if_scalar_and_constant_not_zero(::Any, ::Any) = nothing

@blegat
Copy link
Member Author

blegat commented May 11, 2021

It could but then an invalid call would return nothing instead of a MethodError

@odow
Copy link
Member

odow commented May 11, 2021

How could it get a method error? And when would a method error be preferable?

@blegat
Copy link
Member Author

blegat commented May 11, 2021

If you call it with a scalar function and a vector set. Or with any type like (1, Int)

@odow
Copy link
Member

odow commented May 12, 2021

That should be caught at the original caller? We shouldn't rely on this method to throw a method error. It would violate the MethodError principle.

@blegat
Copy link
Member Author

blegat commented May 12, 2021

Since this is a documented function, the user might call this directly in which case it would get the MethodError at the first method called so it agrees with the principle.
I don't think the places where it is called from the MOI source code will ever never hit the MethodError.

@blegat blegat merged commit 66cbdb6 into master May 12, 2021
@blegat blegat deleted the bl/throw_if_scalar_and_constant_not_zero branch May 12, 2021 01:00
@blegat blegat added this to the v0.10 milestone May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants