Skip to content

neo4j.time.DateTime.now() doesn't accept tzinfo #1103

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

Closed
jackiedeng0 opened this issue Oct 10, 2024 · 2 comments · Fixed by #1104
Closed

neo4j.time.DateTime.now() doesn't accept tzinfo #1103

jackiedeng0 opened this issue Oct 10, 2024 · 2 comments · Fixed by #1104
Labels

Comments

@jackiedeng0
Copy link

Hi, I'm running into the following bug where neo4j.time.DateTime.now() isn't accepting a tzinfo argument:

>>> neo4j.time.DateTime.now(datetime.UTC)
Traceback (most recent call last):
  File "/home/jack/Code/chance-backend/venv/lib/python3.12/site-packages/neo4j/time/__init__.py", line 2206, in now
    return tz.fromutc(  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: fromutc: argument must be a datetime

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jack/Code/chance-backend/venv/lib/python3.12/site-packages/neo4j/time/__init__.py", line 2216, in now
    now_native = tz.fromutc(utc_now_native)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: fromutc: dt.tzinfo is not self

whereas ...Date.today() and ...Time.now() work fine:

>>> neo4j.time.Date.today(datetime.UTC)
neo4j.time.Date(2024, 10, 10)
>>> neo4j.time.Time.now(datetime.UTC)
neo4j.time.Time(12, 45, 8, 60610215, tzinfo=datetime.timezone.utc)

I'm currently using a workaround like this:

neo4j.time.DateTime.utc_now().replace(tzinfo=datetime.UTC)

It might just be caused by some legacy code in ...time/__init__.py, and perhaps some changes like this would make it work and match the other now() and today() functions:

2205,2224c2205,2207
<             try:
<                 return tz.fromutc(  # type: ignore
<                     cls.from_clock_time(  # type: ignore
<                         Clock().utc_time(), UnixEpoch
<                     ).replace(tzinfo=tz)
<                 )
<             except TypeError:
<                 # For timezone implementations not compatible with the custom
<                 # datetime implementations, we can't do better than this.
<                 utc_now = cls.from_clock_time(Clock().utc_time(), UnixEpoch)
<                 utc_now_native = utc_now.to_native()
<                 now_native = tz.fromutc(utc_now_native)
<                 now = cls.from_native(now_native)
<                 return now.replace(
<                     nanosecond=(
<                         now.nanosecond
<                         + utc_now.nanosecond
<                         - utc_now_native.microsecond * 1000
<                     )
<                 )
---
>             return cls.from_clock_time(Clock().local_time(), UnixEpoch)
>                 .replace(tzinfo=timezone.utc)
>                 .astimezone(tz)

I would make a pull request but I've just never worked on this project like this before, thanks.

My Environment

Python Version: 3.
Driver Version: 5.25.0
Server Version and Edition: Neo4j 5
Operating System: Arch Linux

@robsdedude
Copy link
Member

robsdedude commented Oct 11, 2024

Hi and thanks for reaching out.

Looking at the API docs, I find

Temporal data types are implemented by the neo4j.time module.

It provides a set of types compliant with ISO-8601 and Cypher, which are similar to those found in the built-in datetime module. Sub-second values are measured to nanosecond precision and the types are compatible with pytz.

Now this is not very explicit, I find, and I think this should be updated, but it hints at the core of the problem: back when neo4j.time was designed, pytz was the way to go for IANA time zone DB based tzinfo implementations. Python's zoneinfo module was not a thing yet and datetime.timezone was insufficient (only supporting fixed UTC offset time zones).

I suppose making our custom temporal types work with datetime.timezone might be possible. However, it can never work with zoneinfo (new since Python 3.9) since that relies on date times having a fold attribute, which our custom date times don't have. As a matter of fact, I've observed Python segfault when using neo4j.time with zoneinfo. So I highly recommend sticking to pytz (at least for the time being). I still hope that Python manages to get nanosecond precision into its temporal types eventually, which would make all/most of our custom temporal types superfluous.

TL;DR: use pytz timezones and you'll have a much easier time. Other tzinfo implementations might work or might not. We've designed our temporal types with pytz in mind.

@jackiedeng0
Copy link
Author

I see, I appreciate the detailed explanation! I must've overlooked that part in the doc while looking at the table comparing the neo4j.time types to the built-in types. I've changed to pytz and it works fine now, thanks again.

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

Successfully merging a pull request may close this issue.

2 participants