Skip to content

Commit

Permalink
[Bug] Stop coercion of strings into datetime objects (#255)
Browse files Browse the repository at this point in the history
## Problem

Sometimes when strings are stored as metadata values, later fetching or
querying causes those values to be coerced into datetime objects by the
client before they are presented to the caller. This is a default
behavior in the generated openapi client that is a result of the python
`datetime` constructor working quite hard to turn things into dates.
We've seen scenarios where this was not desired, so for now we will
disable this conversion process to have consistent handling of all
strings. People who desire date objects can easily implement the
conversion themselves.

## Solution

- Update openapi templates to comment out and remove logic related to
converting strings into date and datetime objects.
- Regenerate client with new templates
- Add unit test verifying the type coercion does not take place. 

## Type of Change

- [x] Bug fix (non-breaking change which fixes an issue)

## Test Plan

`poetry run pytest tests/unit/data`
  • Loading branch information
jhamon authored Dec 20, 2023
1 parent 5226fcf commit 3792139
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 49 deletions.
96 changes: 47 additions & 49 deletions pinecone/core/client/model_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import re
import tempfile

from dateutil.parser import parse

from pinecone.core.client.exceptions import (
PineconeApiKeyError,
PineconeApiAttributeError,
Expand Down Expand Up @@ -63,7 +61,7 @@ def __get__(self, instance, cls=None):
return result


PRIMITIVE_TYPES = (list, float, int, bool, datetime, date, str, file_type)
PRIMITIVE_TYPES = (list, float, int, bool, str, file_type)

def allows_single_value_input(cls):
"""
Expand Down Expand Up @@ -699,8 +697,8 @@ def __eq__(self, other):
float: 6,
int: 7,
bool: 8,
datetime: 9,
date: 10,
# datetime: 9,
# date: 10,
str: 11,
file_type: 12, # 'file_type' is an alias for the built-in 'file' or 'io.IOBase' type.
}
Expand All @@ -709,8 +707,8 @@ def __eq__(self, other):
# when we have a valid type already and we want to try converting
# to another type
UPCONVERSION_TYPE_PAIRS = (
(str, datetime),
(str, date),
# (str, datetime),
# (str, date),
(int, float), # A float may be serialized as an integer, e.g. '3' is a valid serialized float.
(list, ModelComposed),
(dict, ModelComposed),
Expand Down Expand Up @@ -754,8 +752,8 @@ def __eq__(self, other):
(list, ModelSimple),
# (str, int),
# (str, float),
(str, datetime),
(str, date),
# (str, datetime),
# (str, date),
# (int, str),
# (float, str),
(str, file_type)
Expand Down Expand Up @@ -794,12 +792,12 @@ def get_simple_class(input_value):
return bool
elif isinstance(input_value, int):
return int
elif isinstance(input_value, datetime):
# this must be higher than the date check because
# isinstance(datetime_instance, date) == True
return datetime
elif isinstance(input_value, date):
return date
# elif isinstance(input_value, datetime):
# # this must be higher than the date check because
# # isinstance(datetime_instance, date) == True
# return datetime
# elif isinstance(input_value, date):
# return date
elif isinstance(input_value, str):
return str
return type(input_value)
Expand Down Expand Up @@ -1210,42 +1208,42 @@ def deserialize_primitive(data, klass, path_to_item):
:param data: str/int/float
:param klass: str/class the class to convert to
:return: int, float, str, bool, date, datetime
:return: int, float, str, bool
"""
additional_message = ""
try:
if klass in {datetime, date}:
additional_message = (
"If you need your parameter to have a fallback "
"string value, please set its type as `type: {}` in your "
"spec. That allows the value to be any type. "
)
if klass == datetime:
if len(data) < 8:
raise ValueError("This is not a datetime")
# The string should be in iso8601 datetime format.
parsed_datetime = parse(data)
date_only = (
parsed_datetime.hour == 0 and
parsed_datetime.minute == 0 and
parsed_datetime.second == 0 and
parsed_datetime.tzinfo is None and
8 <= len(data) <= 10
)
if date_only:
raise ValueError("This is a date, not a datetime")
return parsed_datetime
elif klass == date:
if len(data) < 8:
raise ValueError("This is not a date")
return parse(data).date()
else:
converted_value = klass(data)
if isinstance(data, str) and klass == float:
if str(converted_value) != data:
# '7' -> 7.0 -> '7.0' != '7'
raise ValueError('This is not a float')
return converted_value
# if klass in {datetime, date}:
# additional_message = (
# "If you need your parameter to have a fallback "
# "string value, please set its type as `type: {}` in your "
# "spec. That allows the value to be any type. "
# )
# if klass == datetime:
# if len(data) < 8:
# raise ValueError("This is not a datetime")
# # The string should be in iso8601 datetime format.
# parsed_datetime = parse(data)
# date_only = (
# parsed_datetime.hour == 0 and
# parsed_datetime.minute == 0 and
# parsed_datetime.second == 0 and
# parsed_datetime.tzinfo is None and
# 8 <= len(data) <= 10
# )
# if date_only:
# raise ValueError("This is a date, not a datetime")
# return parsed_datetime
# elif klass == date:
# if len(data) < 8:
# raise ValueError("This is not a date")
# return parse(data).date()
# else:
converted_value = klass(data)
if isinstance(data, str) and klass == float:
if str(converted_value) != data:
# '7' -> 7.0 -> '7.0' != '7'
raise ValueError('This is not a float')
return converted_value
except (OverflowError, ValueError) as ex:
# parse can raise OverflowError
raise PineconeApiValueError(
Expand Down Expand Up @@ -1782,7 +1780,7 @@ def get_oneof_instance(cls, model_kwargs, constant_kwargs, model_arg=None):
and path to item.
Kwargs:
model_arg: (int, float, bool, str, date, datetime, ModelSimple, None):
model_arg: (int, float, bool, str, ModelSimple, None):
the value to assign to a primitive class or ModelSimple class
Notes:
- this is only passed in when oneOf includes types which are not object
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/data/test_datetime_parsing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from pinecone import Vector, Config
from datetime import datetime

class TestDatetimeConversion:
def test_datetimes_not_coerced(self):
vec = Vector(id='1', values=[0.1, 0.2, 0.3], metadata={'created_at': '7th of January, 2023'}, _check_type=True, _configuration=Config())
assert vec.metadata['created_at'] == '7th of January, 2023'
assert vec.metadata['created_at'].__class__ == str

def test_dates_not_coerced(self):
vec = Vector(id='1', values=[0.1, 0.2, 0.3], metadata={'created_at': '8/12/2024'}, _check_type=True, _configuration=Config())
assert vec.metadata['created_at'] == '8/12/2024'
assert vec.metadata['created_at'].__class__ == str

0 comments on commit 3792139

Please sign in to comment.