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

nextprod with vector containing 1 gives DomainError #20410

Closed
KristofferC opened this issue Feb 2, 2017 · 8 comments
Closed

nextprod with vector containing 1 gives DomainError #20410

KristofferC opened this issue Feb 2, 2017 · 8 comments
Labels
error handling Handling of exceptions by Julia or the user

Comments

@KristofferC
Copy link
Member

julia> nextprod([1, 2], 10)
ERROR: DomainError:
Stacktrace:
 [1] nextpow(::Int64, ::Int64) at ./intfuncs.jl:284
 [2] collect(::Base.Generator{Array{Int64,1},Base.##410#411{Int64}}) at ./array.jl:389
 [3] nextprod(::Array{Int64,1}, ::Int64) at ./combinatorics.jl:210

This error could be better.

@KristofferC KristofferC added the error handling Handling of exceptions by Julia or the user label Feb 2, 2017
@yuyichao
Copy link
Contributor

yuyichao commented Feb 2, 2017

Dup of #12152

@yuyichao yuyichao closed this as completed Feb 2, 2017
@KristofferC
Copy link
Member Author

Not really.. could throw an argument error here.

@yuyichao
Copy link
Contributor

yuyichao commented Feb 2, 2017

Well, I think a DomainError is a perfectly good exception to use for this. Also, using an ArgumentError won't help with the performance issue at all and that's essentially what #12152 is about.

@KristofferC
Copy link
Member Author

KristofferC commented Feb 2, 2017

This function seems big enough for the performance problem to matter. It even allocates a temp array. Note, I am saying this could be caught in nextprod and not nextpow

@yuyichao
Copy link
Contributor

yuyichao commented Feb 2, 2017

It seems like a bad idea to decide if a DomainError or an ArgumentError should be used based on how expensive the function is.

@KristofferC
Copy link
Member Author

KristofferC commented Feb 2, 2017

You were the one who brought up performance problems. I just argued that performance of exceptions was not relevant here.

@yuyichao
Copy link
Contributor

yuyichao commented Feb 2, 2017

I mean I don't think we should switching away from DomainError just for this. And in case you want to do it more generally, it'll still be the same issue as #12152 even though it's not about DomainError anymore.

@KristofferC
Copy link
Member Author

OK, added a comment there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

No branches or pull requests

3 participants