-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Deprecate DateTime() Date() Time() #23724
Conversation
Commits should be squashed upon merging. |
base/dates/types.jl
Outdated
DateTime() = now() | ||
Date() = today() | ||
Time() = Time(now()) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have to update the documentation after the release of 0.7.
This will cause
during sysimg build and julia> DateTime()
WARNING: DateTime() is deprecated, use DateTime(1) instead.
Stacktrace:
[1] depwarn(::String, ::Symbol) at ./deprecated.jl:68
[2] DateTime() at ./deprecated.jl:56
[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] (::getfield(Base.REPL, Symbol("##1#2")){Base.REPL.REPLBackend})() at ./event.jl:96
while loading no file, in expression starting on line 0
0001-01-01T00:00:00 on master as soon as the version is bumped. Would it be better to add the new methods in a separate PR after the deprecations are removed? Then the docs could be updated in said PR also, so we don't forget about it? |
@frederikekre's plan sounds like a good one to me. @a-hofer, if you're willing to do that, please go ahead, otherwise, @fredrikekre, could you push commits to this branch to "make it so"? |
2824ac7
to
bb8a604
Compare
Okay, I have fixed up the PR. But as I did so I thought about two things:
It is clearly documented what the periods default to, With this in mind I don't think this PR is accomplishing anything really, and I argue that status quo is the best. |
bb8a604
to
c3db3da
Compare
One of the issues here is that the |
If we decide not to go with |
For reference: MATLAB's datenum and datevec do not have 0-arg forms. MATLAB's new datetime does, and it means the current date and time. |
For context I am just learning Julia, and I wanted to create a 'now' DateTime, and as @omus mentioned it was not obvious how to do so from the documentation. Personally I think it would be much more helpful if the empty constructor had this functionality, because this seems like a very common use case to me, at least much more common than needing DateTime(1). Whatever you guys think is best is fine with me, just figured you might want a newbie perspective. Mentioning the now() function in the documentation might have a similar effect. |
I was actually thinking that it might make sense and go completely the other direction: deprecate |
I think I will leave this PR in the hands of |
By the way, currently |
Bump --- do we want this in 1.0? Needs to be rebased after Dates was moved to stdlib. |
d85c4d5
to
110f741
Compare
Will be `now` in future release.
110f741
to
3b8c9d3
Compare
Changes have been rebased to use the stdlib Dates. I'm in favor of adding this deprecation but I thought it may be best for triage to discuss. |
No triage required, we should do this. |
Will be
now
after julia 0.7.