-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
@@ -94,9 +99,7 @@ def _build_dataframe(time_series_iterable, label=None, labels=None): # pragma: | |||
for time_series in time_series_iterable: | |||
pandas_series = pandas.Series( | |||
data=[_extract_value(point.value) for point in time_series.points], | |||
index=[ | |||
point.interval.end_time.ToNanoseconds() for point in time_series.points |
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.
Ah what this seems to do is convert the time to just nanoseconds... https://proto-plus-python.readthedocs.io/en/latest/reference/datetime_helpers.html#proto.datetime_helpers.DatetimeWithNanoseconds
tests/unit/test__dataframe.py
Outdated
from google.api_core import datetime_helpers | ||
|
||
return [datetime_helpers.from_rfc3339(t).replace(tzinfo=None) for t in TIMESTAMPS] | ||
return [pandas.Timestamp(t) for t in TIMESTAMPS] |
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.
pandas seems to have its own timestamp type (https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Timestamp.html), so I'm not sure how these tests used to work 🤔
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.
LGTM. Just one minor observation.
The dataframe helper methods were being skipped because
pandas
was not installed in the unit test session. This re-enables those tests and makes the minimal number of fixes to get the unit tests passing again.TODOs:
WhichOneOf
with proto-plus? Document how to usemodule_pb2.Message.WhichOneof
proto-plus-python#137Fixes #90 🦕
Simple example: