-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Add timestamp method+test; closes #17329 #17906
Conversation
doc/source/whatsnew/v0.21.0.txt
Outdated
@@ -235,6 +235,7 @@ Other Enhancements | |||
- Improved the import time of pandas by about 2.25x. (:issue:`16764`) | |||
- :func:`read_json` and :func:`to_json` now accept a ``compression`` argument which allows them to transparently handle compressed files. (:issue:`17798`) | |||
- :func:`Series.reindex`, :func:`DataFrame.reindex`, :func:`Index.get_indexer` now support list-like argument for ``tolerance``. (:issue:`17367`) | |||
- :meth:`Timestamp.timestamp` is now available in python 2.7. (:issue:`17329`) |
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.
Nitpick: All other instances of "Python" in the whatsnew have a capitalized P.
pandas/_libs/tslib.pyx
Outdated
@@ -1405,6 +1404,10 @@ cdef class _Timestamp(datetime): | |||
def __get__(self): | |||
return np.datetime64(self.value, 'ns') | |||
|
|||
def timestamp(self): | |||
# py27 compat, see GH#17329 | |||
return self.value / 1e9 |
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 doc-string
def test_timestamp(self): | ||
# Gh#17329 | ||
# tz-naive --> treat it as if it were UTC for purposes of timestamp() | ||
ts = Timestamp.now() |
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.
test versus a datetime.timestamp() value here (as well)
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.
e.g. ts.to_pydatetime().timestamp()
Codecov Report
@@ Coverage Diff @@
## master #17906 +/- ##
==========================================
- Coverage 91.23% 91.22% -0.02%
==========================================
Files 163 163
Lines 50105 50105
==========================================
- Hits 45715 45706 -9
- Misses 4390 4399 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17906 +/- ##
==========================================
+ Coverage 91.24% 91.25% +0.01%
==========================================
Files 163 163
Lines 50173 50173
==========================================
+ Hits 45778 45786 +8
+ Misses 4395 4387 -8
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.21.0.txt
Outdated
@@ -235,6 +235,7 @@ Other Enhancements | |||
- Improved the import time of pandas by about 2.25x. (:issue:`16764`) | |||
- :func:`read_json` and :func:`to_json` now accept a ``compression`` argument which allows them to transparently handle compressed files. (:issue:`17798`) | |||
- :func:`Series.reindex`, :func:`DataFrame.reindex`, :func:`Index.get_indexer` now support list-like argument for ``tolerance``. (:issue:`17367`) | |||
- :meth:`Timestamp.timestamp` is now available in Python 2.7. (:issue:`17329`) |
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.
is this in api.rst?
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.
It is now.
# utsc is a different representation of the same time | ||
assert tsc.timestamp() == utsc.timestamp() | ||
|
||
if PY3: |
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.
is this not available in py2? on datetime?
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.
Correct. The method was added to the stdlib datetime.datetime in py3.3
uts = ts.replace(tzinfo=utc) | ||
assert ts.timestamp() == uts.timestamp() | ||
|
||
tsc = Timestamp('2014-10-11 11:00:01.12345678', tz='US/Central') |
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 u maker sure test for mat is ok?
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 don't understand the question.
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.
for NaT
dt = ts.to_pydatetime() | ||
assert dt.timestamp() == round(ts.timestamp(), 6) | ||
|
||
pytest.raises(ValueError, NaT.timestamp) |
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 move this to the section where we test NaT (test_nat.py)
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.
Sure.
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.
Come to think of it, should NaT.timestamp raise or return nan?
if PY3: | ||
# should agree with datetime.timestamp method | ||
dt = ts.to_pydatetime() | ||
assert dt.timestamp() == round(ts.timestamp(), 6) |
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.
why do you need to round?
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.
Because dt.timestamp()
is the datetime.timestamp
method which only has microsecond-precision.
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.
exactly, you should not need to do this
pandas/_libs/tslib.pyx
Outdated
def timestamp(self): | ||
"""Return POSIX timestamp as float.""" | ||
# py27 compat, see GH#17329 | ||
return self.value / 1e9 |
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 is wrong if we have a nanosecond component. should issue a warning if we have one (I think its a print statement, but that's consistent with what we have now). add a test for this.
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.
Why is it wrong with a nanosecond component? Is it a precision issue?
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.
yes its not compatible with datetime.timestamp() (since you cannot construct one in datetime, we should not allow a time value which is invalid)
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'll make this change, will add follow-up to the tslibs todo list, since it isn't obvious to me that this is the correct decision.
Please remove me from the list
On Tue, Oct 17, 2017 at 7:53 PM jbrockmendel ***@***.***> wrote:
- closes #17329 <#17329>
- tests added / passed
- passes git diff upstream/master -u -- "*.py" | flake8 --diff
- whatsnew entry
------------------------------
You can view, comment on, or merge this pull request online at:
#17906
Commit Summary
- Add timestamp method+test; closes #17329
- whatsnew entry
File Changes
- *M* doc/source/whatsnew/v0.21.0.txt
<https://github.com/pandas-dev/pandas/pull/17906/files#diff-0> (1)
- *M* pandas/_libs/tslib.pyx
<https://github.com/pandas-dev/pandas/pull/17906/files#diff-1> (6)
- *M* pandas/tests/scalar/test_timestamp.py
<https://github.com/pandas-dev/pandas/pull/17906/files#diff-2> (12)
Patch Links:
- https://github.com/pandas-dev/pandas/pull/17906.patch
- https://github.com/pandas-dev/pandas/pull/17906.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#17906>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJfaxH6VMPu__Vl7ywaSM3mPR7NsBb7ks5stOkfgaJpZM4P8i8g>
.
--
Greg Gurevich
greggurevich@gmail.com
cell: 1-917-504-7105
|
pandas/tests/scalar/test_nat.py
Outdated
@@ -123,6 +123,11 @@ def test_round_nat(klass): | |||
assert round_method(freq) is ts | |||
|
|||
|
|||
def test_timestamp(): |
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.
pls just add this where the others are (IOW in the list), don't special case this.
try. test where u take a min zero nanosecond Timestamp then do .timestamp() and create this as a datetime it will lose the nanoseconds |
thanks |
(cherry picked from commit 5dd2ea0)
git diff upstream/master -u -- "*.py" | flake8 --diff