-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Make Timestamp('now') equivalent to Timestamp.now() #9022
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
Conversation
# Check that the delta between the times is less than 1s (arbitrarily small) | ||
delta = Timedelta(seconds=1) | ||
self.assertLess((ts_from_method - ts_from_string), delta) | ||
self.assertLess((ts_from_method_tz - ts_from_string_tz), delta) |
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.
I think this will break 2.6 (not sure if we implemented this method), so use assertTrue(value1 < value2)
@@ -301,6 +301,28 @@ def test_barely_oob_dts(self): | |||
def test_utc_z_designator(self): | |||
self.assertEqual(get_timezone(Timestamp('2014-11-02 01:00Z').tzinfo), 'UTC') | |||
|
|||
def test_now(self): | |||
# #9000 |
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.
can you add a test with a tz (just for consistency)
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.
today() doesn't take a tz argument so I left that particular test out.
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.
hmm, if one is passed what should happen? raise?
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.
Well now that I think about it I see no reason why a tz shouldn't be an allowed argument. Maybe just have today() call now() and then we are done.
2251113
to
ca96d87
Compare
return cls(datetime.today()) | ||
def today(cls, tz=None): | ||
""" | ||
Return the current time in the local timezone. This differs |
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.
This docstring needs an update now (and should probably be the same as the one for today
).
…day') equivalent to Timestamp.today() and pass tz to today().
this looks good @shoyer @jorisvandenbossche any other comments? |
Looks good to me |
@rockg thanks! |
closes #9000
I opted to not change np_datetime_strings but instead short-circuit in Timestamp. I don't have a strong preference one way or another.