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

Timestamp not handled correctly #10

Closed
racinmat opened this issue Jan 25, 2024 · 3 comments · Fixed by #11
Closed

Timestamp not handled correctly #10

racinmat opened this issue Jan 25, 2024 · 3 comments · Fixed by #11
Labels
bug Something isn't working

Comments

@racinmat
Copy link

racinmat commented Jan 25, 2024

I have following issue with timestamp and mypy:

mwe.py

import proto
from google.protobuf import timestamp_pb2


class MyClass(proto.Message):
    create_time = proto.Field(
        proto.MESSAGE,
        number=6,
        message=timestamp_pb2.Timestamp,
    )


if __name__ == '__main__':
    classes = [MyClass(create_time=timestamp_pb2.Timestamp(seconds=1, nanos=1)),
               MyClass(create_time=timestamp_pb2.Timestamp(seconds=2, nanos=2))]

    print(f"{max(classes, key=lambda m: m.create_time)=}")

when I run the code, it succeeds:

python .\mwe.py            
max(classes, key=lambda m: m.create_time)=create_time {
  seconds: 2
  nanos: 2
}

but it fails on mypy

mypy .\mwe.py
mwe.py:17: error: Argument "key" to "max" has incompatible type "Callable[[MyClass], Timestamp]"; expected "Callable[[MyClass], Union[SupportsDunderLT[Any], SupportsDunderGT[Any]]]"  [arg-type]
mwe.py:17: error: Incompatible return value type (got "Timestamp", expected "Union[SupportsDunderLT[Any], SupportsDunderGT[Any]]")  [return-value]
Found 2 errors in 1 file (checked 1 source file)

I hope this is the right package, because this is an issue between mypy, protobuf, proto-plus and maybe something else.
When I have a Timestamp as a field, the proto-plus turns it into DatetimeWithNanoseconds which has arithmetic defined, because it extends datetime.datetime, but mypy does not know about it.
This is more elaborate example

problem_example.py

"""
conda create -n mypy_experiments python=3.9
conda activate mypy_experiments
"""
from datetime import datetime

import proto
from google.protobuf import timestamp_pb2


class MyClass(proto.Message):
    create_time = proto.Field(
        proto.MESSAGE,
        number=6,
        message=timestamp_pb2.Timestamp,
    )


if __name__ == '__main__':
    classes = [MyClass(create_time=timestamp_pb2.Timestamp(seconds=1, nanos=1)),
               MyClass(create_time=timestamp_pb2.Timestamp(seconds=2, nanos=2))]

    print(f"{max(classes, key=lambda m: m.create_time)=}")

    times = [d.create_time for d in classes]
    print(f"{max(times)=}")
    print(f"{type(times[0])=}, {isinstance(times[0], datetime)=}")
    times2 = [timestamp_pb2.Timestamp(seconds=1, nanos=1), timestamp_pb2.Timestamp(seconds=2, nanos=2)]
    print(f"{type(times2[0])=}, {isinstance(times2[0], datetime)=}")
    try:
        print(f"{max(times2)=}")
    except Exception as e:
        print(e)
    print(f"{max(times, key=lambda t: t)=}")
    try:
        print(f"{max(times2, key=lambda t: t)=}")
    except Exception as e:
        print(e)

which prints

python .\problem_example.py
max(classes, key=lambda m: m.create_time)=create_time {
  seconds: 2
  nanos: 2
}

max(times)=DatetimeWithNanoseconds(1970, 1, 1, 0, 0, 2, tzinfo=datetime.timezone.utc)
type(times[0])=<class 'proto.datetime_helpers.DatetimeWithNanoseconds'>, isinstance(times[0], datetime)=True
type(times2[0])=<class 'google.protobuf.timestamp_pb2.Timestamp'>, isinstance(times2[0], datetime)=False
'>' not supported between instances of 'Timestamp' and 'Timestamp'
max(times, key=lambda t: t)=DatetimeWithNanoseconds(1970, 1, 1, 0, 0, 2, tzinfo=datetime.timezone.utc)
'>' not supported between instances of 'Timestamp' and 'Timestamp'

I am using python 3.9 and here are all versions of packages:

pip freeze
filelock==3.13.1
mypy==1.8.0
mypy-extensions==1.0.0
mypy-protobuf==3.5.0
proto-plus==1.23.0
proto-plus-stubs==0.6.0        
protobuf==4.25.2
requests-file==1.5.1
tldextract==5.1.1
tomli==2.0.1
types-protobuf==4.24.0.20240106
typing_extensions==4.9.0       
XlsxWriter==3.1.9
@henribru
Copy link
Owner

This is indeed an issue with this package. There's no special-handling of Timestamp currently. I'll see if I can do something about that. Thanks for the heads-up

@henribru
Copy link
Owner

This is fixed in v0.6.1.

@racinmat
Copy link
Author

Thanks! this is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants