Skip to content
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

Clarification needed on zero_time() function behavior and related test failures #156

Open
riskassessor opened this issue Aug 18, 2024 · 5 comments

Comments

@riskassessor
Copy link

Hello PyGNOME maintainers,

I've recently installed PyGNOME and encountered two failing unit tests related to the zero_time() function in time_utils.py. The tests seem to expect zero_time() to return a value corresponding to a date in 2106, but it's actually returning the current timezone offset.

The current implementation of zero_time() is:
def zero_time(): offset = timezone_offset_seconds() return offset if offset >= 0 else 0

And timezone_offset_seconds() is
def timezone_offset_seconds(): return time.mktime(time.localtime()) - time.mktime(time.gmtime())

The failing tests are:

test_datetime_array in test_time_utils.py
test_numpy_array in test_time_utils.py

Could you please clarify:

What is the intended behavior of zero_time()? Should it return a fixed "zero time" (e.g., 2106-02-07T05:28:16) or the current timezone offset?
If the current implementation is correct, how should the tests be updated to reflect this?
If the implementation needs to change, what should be the correct implementation?

Thank you for your help in resolving this issue.

@ChrisBarker-NOAA
Copy link
Contributor

We are not seeing that test failure here.

What versions of:

Operation System

Compiler

Python

PyGNOME

are you using?

And what timezone is your machine set to?

(PyGNOME not so important, that code hasn't changed in years ...)

That function is use only for interacting with the C code that handles time -- unfortunately, the old C time library applies the Locale timezone offset automatically, so in our code, we need to mange that to get non-locale specific times.

I think the goal of zero_time() function is to return the number of seconds since the epoch in the locale's timezone -- it may not be zero if the timezone is not UTC.

Does your machine have the timezone set to a positive offset? It may be that we have only tested in the western hemisphere, where the offset is always negative, which would explain the code:

return offset if offset >= 0 else 0

However, the docstring of timezone_offset_seconds indicates that it's supposed to be for teh eastern hemisphere.

@coconnor8: Any ideas?

@coconnor8
Copy link
Contributor

I think that utcfromtimestamp can't handle negative values. That is only used in those two tests.

@ChrisBarker-NOAA
Copy link
Contributor

I've done a bit more testing -- I set my machine to a timezone in the eastern hemisphere.

zero-time() returns a negative number.

The tests all still pass for me.

datetime.utcfromtimestamp()

Works fine with a negative number on my machine.

MacOS, Python 3.10.

So until we figure out why it's failing for @riskassessor -- not sure what can be done.

@ChrisBarker-NOAA
Copy link
Contributor

What is the intended behavior of zero_time()? Should it return a fixed "zero time" (e.g., 2106-02-07T05:28:16) or the current timezone offset?

It's supposed to return a timestamp in seconds that corresponds to the minimum time in the current time zone -- usually 1/1/1970 - the timezone offset, if negative.

thinking now -- the zero_time() probably shouldn't be used in those tests -- that's not what's being tested.

@riskassessor
Copy link
Author

We are not seeing that test failure here.
What versions of:
Operation System
Compiler
Python
PyGNOME
are you using?
And what timezone is your machine set to?

OS: MacOS Sonoma 14.5
Compiler: I don't think I compiled anything. I installed PyGNOME and dependencies using conda.
Python: 3.10.14
PyGNOME: 1.1.13
Time zone: British Summer Time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants