MideTechnology / endaq-python

A comprehensive, user-centric Python API for working with enDAQ data and devices
MIT License
24 stars 12 forks source link

Feature/118 pandas localtime #184

Closed StokesMIDE closed 2 years ago

StokesMIDE commented 2 years ago

This is a solution to issue #118. It adds a tz parameter, which can be the strings "utc" (default), "local" (user's local time), "device" (TZ based on recorder's UTC offset), or a standard type of time zone info object.

One significant change is that the index for all "datetime" indices, the type was explicitly changed to a pandas.DatetimeIndex in order to add the timezone information (numpy.datetime64 does not support timezones). We will have to be on the lookout for any issues this might introduce (small discrepancies due to rounding errors or whatnot).

Note: current tests are minimal. Mostly just a sanity check.

StokesMIDE commented 2 years ago

Only comment is that colab's local time seems to be UTC?

I expected something like that. The Python interpreter isn't running locally (it's on a server that could be anywhere in the world) and it doesn't know the local system time. I guess it could have used geolocation to guess, but that seems overly complicated (and not totally reliable). Use in enDAQ Cloud will probably demonstrate the same. I made a note of this in the docstring.

codecov-commenter commented 2 years ago

Codecov Report

Merging #184 (23ecb52) into development (52778a2) will increase coverage by 0.20%. The diff coverage is 80.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #184      +/-   ##
===============================================
+ Coverage        61.72%   61.93%   +0.20%     
===============================================
  Files               34       34              
  Lines             2613     2643      +30     
===============================================
+ Hits              1613     1637      +24     
- Misses            1000     1006       +6     
Impacted Files Coverage Δ
endaq/ide/info.py 86.18% <80.00%> (-1.82%) :arrow_down:
endaq/calc/utils.py 86.04% <0.00%> (+1.04%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 52778a2...23ecb52. Read the comment docs.

StokesMIDE commented 2 years ago

@CrepeGoat You've used np.datetime64 in various things; do you foresee any issues with using the Pandas date times? I'm pretty sure using np.datetime64 as a dataframe index implicitly creates a pandas.DatetimeIndex, anyway.

CrepeGoat commented 2 years ago

@CrepeGoat You've used np.datetime64 in various things; do you foresee any issues with using the Pandas date times? I'm pretty sure using np.datetime64 as a dataframe index implicitly creates a pandas.DatetimeIndex, anyway.

Yeah that's my understanding as well. So it might change behavior outside of setting DataFrame indices, but any behavioral difference besides like, isinstance checks I'm not especially familiar with either. That's generally why I opted for np.dateime64. But as long as tests are passing it should be good?

...¯_(ツ)_/¯. hope that helps.