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

TODO after 0.7: DomainError() should be equivalent to DomainError(nothing) #23132

Closed
musm opened this issue Aug 4, 2017 · 10 comments
Closed

Comments

@musm
Copy link
Contributor

musm commented Aug 4, 2017

julia> DomainError()
WARNING: DomainError now supports arguments, use `DomainError(value)` or `DomainError(value, msg)` instead.
Stacktrace:
 [1] depwarn at .\deprecated.jl:70 [inlined]
 [2] DomainError() at .\deprecated.jl:1550
 [3] eval(::Module, ::Expr) at .\repl\REPL.jl:3
 [4] eval_user_input(::Any, ::Base.REPL.REPLBackend) at .\repl\REPL.jl:69
 [5] macro expansion at .\repl\REPL.jl:100 [inlined]
 [6] (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at .\event.jl:73
while loading no file, in expression starting on line 0
DomainError(nothing, #undef)

IMO this should not throw a warning . Was this intentional so that package authors start using the new feature?

@fredrikekre
Copy link
Member

It is an intentional deprecation warning, see #22751

@musm
Copy link
Contributor Author

musm commented Aug 5, 2017

I don't see a discussion regarding the deprecation warning in the issue?

@fredrikekre
Copy link
Member

What do you mean? Zero-arg DomainError() was deprecated in the linked PR.

julia/base/deprecated.jl

Lines 1544 to 1548 in 83007fb

# PR #22751
function DomainError()
depwarn("DomainError now supports arguments, use `DomainError(value)` or `DomainError(value, msg)` instead.", :DomainError)
DomainError(nothing)
end

@musm
Copy link
Contributor Author

musm commented Aug 6, 2017

RIght. My point is that I think DomainError() --> DomainError(nothing) i.e the previous behavior should just correspond to what is currently DomainError(nothing) .

@musm musm changed the title DomainError() with no arguments throws warning DomainError() should be equivalent to DomainError(nothing) Aug 6, 2017
@fredrikekre
Copy link
Member

Why? The point of #22751 was to make DomainError more informative; so why would you opt out of that new feature?

@musm
Copy link
Contributor Author

musm commented Aug 6, 2017

The new feature is great. I have no problems with it.
For error, you can opt in to adding a more descriptive error message or you can decide to leave it as error() . With DomainError, perhaps you do not care to add a message then you have to use DomainError(nothing) currently, instead of DomainError() which is what I am suggesting it be an alias for.

Considering the misunderstanding, I think this should be reopened.

@musm
Copy link
Contributor Author

musm commented Aug 6, 2017

@timholy : Since you implemented this feature, care to share any insight on this?

@timholy
Copy link
Member

timholy commented Aug 6, 2017

Perhaps unsurprisingly, I agree with @fredrikekre ---without a deprecation warning, people won't change their code, and consequently we'll still have impenetrable errors in production code. I agree that for "quick scripting" for your own personal use, there might be times when you don't want to bother being explicit about the nature of the problem, but I think that's a minor disadvantage compared to the benefits.

The only reason it wasn't worse before was that we used to do something clever to try to be at least a bit informative, but I think that being stupid is much better than being clever when you can get away with it (and now we can).

After 1.0 comes around we could consider adding the fallback you're asking for, but because the next release would be the first (only) with a depwarn I think we have to leave it the way it is now.

@musm
Copy link
Contributor Author

musm commented Aug 6, 2017

@timholy Thanks for the clarification. I think it would then be best to add something to Compat to allow using this feature in a backwards compatible manner with 0.6 .
I think this issue should receive a TODO or pending label.

@timholy
Copy link
Member

timholy commented Aug 6, 2017

Good idea (and I should have thought of this myself). I am pretty swamped for the next couple of days, so if you're in a hurry I'd appreciate some help. But I will put this on my TODO list if no one beats me to it.

@musm musm changed the title DomainError() should be equivalent to DomainError(nothing) TODO after 0.7: DomainError() should be equivalent to DomainError(nothing) Aug 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants