Skip to content

[AL-5784] Remove backports because it failed to installed on Windows #1119

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jun 6, 2023
Merged

[AL-5784] Remove backports because it failed to installed on Windows #1119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fd6231b
Use python dateutil package to parse iso strings
May 30, 2023
973016b
Remove backports
May 30, 2023
9aaeb72
Fix formatting
May 30, 2023
b78fbfb
Make tox.ini uptodate with supported python versions
May 30, 2023
4d25d85
Rename function
May 30, 2023
b2f348d
Add test
May 30, 2023
541d7c4
PR changes
Jun 5, 2023
2924dc9
Support for timezone conversion
Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions labelbox/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
name = "labelbox"
__version__ = "3.47.1"

from backports.datetime_fromisoformat import MonkeyPatch

MonkeyPatch.patch_fromisoformat()

from labelbox.client import Client
from labelbox.schema.project import Project
from labelbox.schema.model import Model
Expand Down
6 changes: 3 additions & 3 deletions labelbox/schema/data_row_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pydantic import BaseModel, conlist, constr

from labelbox.schema.ontology import SchemaId
from labelbox.utils import _CamelCaseMixin, format_iso_datetime
from labelbox.utils import _CamelCaseMixin, format_iso_datetime, format_iso_from_string


class DataRowMetadataKind(Enum):
Expand Down Expand Up @@ -466,7 +466,7 @@ def parse_metadata_fields(
value=schema.uid)
elif schema.kind == DataRowMetadataKind.datetime:
field = DataRowMetadataField(schema_id=schema.uid,
value=datetime.fromisoformat(
value=format_iso_from_string(
f["value"]))
else:
field = DataRowMetadataField(schema_id=schema.uid,
Expand Down Expand Up @@ -838,7 +838,7 @@ def _validate_parse_number(
def _validate_parse_datetime(
field: DataRowMetadataField) -> List[Dict[str, Union[SchemaId, str]]]:
if isinstance(field.value, str):
field.value = datetime.fromisoformat(field.value)
field.value = format_iso_from_string(field.value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From line 849, we take the field.value and convert it into string from the datetime using format_iso_datetime. Since format_iso_datetime does not take into consideration the timezone, does that mean that the timezone information will get lost here?

For instance, if user inputs 2011-11-04T00:05:23+05:00 as the date metadata, it looks like we'll be upserting 2011-11-04T00:05:23 and the timezone value gets lost? Let me know if I'm not understanding this correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time zone does get lost! This is existing before... that was my question - should I fix it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know the original issue was to make sure the time string gets properly parsed if timezone passed in.. we should make this also properly parse the timezone, because otherwise, non-UTC timezones will not store accurate datetimes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok... to be clear format_iso_from_string is not a problem, as far as I remember from my testing format_iso_datetime is the problem, it just formats a string with a Z time zone, no conversion. Those two functions are used together in the code (like to validate string is a valid datetime AND convert it back to string)

elif not isinstance(field.value, datetime):
raise TypeError(
f"Value for datetime fields must be either a string or datetime object. Found {type(field.value)}"
Expand Down
18 changes: 17 additions & 1 deletion labelbox/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import datetime
import re

from dateutil.tz import tzoffset
from dateutil.parser import isoparse as dateutil_parse
from dateutil.utils import default_tzinfo

from urllib.parse import urlparse
from pydantic import BaseModel

UPPERCASE_COMPONENTS = ['uri', 'rgb']
ISO_DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
DFLT_TZ = tzoffset("UTC", 0000)


def _convert(s, sep, title):
Expand Down Expand Up @@ -80,4 +86,14 @@ def format_iso_datetime(dt: datetime.datetime) -> str:
Formats a datetime object into the format: 2011-11-04T00:05:23Z
Note that datetime.isoformat() outputs 2011-11-04T00:05:23+00:00
"""
return dt.strftime(ISO_DATETIME_FORMAT)
return dt.astimezone(datetime.timezone.utc).strftime(ISO_DATETIME_FORMAT)


def format_iso_from_string(date_string: str) -> datetime.datetime:
"""
Converts a string even if offset is missing: 2011-11-04T00:05:23Z or 2011-11-04T00:05:23+00:00 or 2011-11-04T00:05:23
to a datetime object.
For missing offsets, the default offset is UTC.
"""
# return datetime.datetime.fromisoformat(date_string)
return default_tzinfo(dateutil_parse(date_string), DFLT_TZ)
24 changes: 12 additions & 12 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
requests==2.22.0
backoff==1.10.0
google-api-core>=1.22.1
pydantic>=1.8,<2.0
shapely
tqdm
geojson
google-api-core>=1.22.1
imagesize
nbconvert~=7.2.6
nbformat~=5.7.0
numpy
PILLOW
opencv-python
imagesize
pyproj
PILLOW
pydantic>=1.8,<2.0
pygeotile
typing-extensions==4.5.0
pyproj
pytest-xdist
nbformat~=5.7.0
nbconvert~=7.2.6
python-dateutil>=2.8.2,<2.9.0
requests==2.22.0
shapely
tqdm
typeguard==2.13.3
backports-datetime-fromisoformat~=2.0
typing-extensions==4.5.0
3 changes: 1 addition & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
packages=setuptools.find_packages(),
install_requires=[
"backoff==1.10.0", "requests>=2.22.0", "google-api-core>=1.22.1",
"pydantic>=1.8,<2.0", "tqdm", "backports-datetime-fromisoformat~=2.0"
"pydantic>=1.8,<2.0", "tqdm", "python-dateutil>=2.8.2,<2.9.0"
],
extras_require={
'data': [
Expand All @@ -31,7 +31,6 @@
],
},
classifiers=[
'Development Status :: 3 - Alpha',
'License :: OSI Approved :: Apache Software License',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.7',
Expand Down
9 changes: 4 additions & 5 deletions tests/integration/test_data_row_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ def test_delete_schema(mdo):


@pytest.mark.parametrize('datetime_str',
['2011-11-04T00:05:23Z', '2011-05-07T14:34:14+00:00'])
['2011-11-04T00:05:23Z', '2011-11-04T00:05:23+00:00'])
def test_upsert_datarow_date_metadata(data_row, mdo, datetime_str):
metadata = [
DataRowMetadata(data_row_id=data_row.uid,
Expand All @@ -458,11 +458,11 @@ def test_upsert_datarow_date_metadata(data_row, mdo, datetime_str):
assert len(errors) == 0

metadata = mdo.bulk_export([data_row.uid])
assert metadata[0].fields[0].value == datetime.fromisoformat(datetime_str)
assert f"{metadata[0].fields[0].value}" == "2011-11-04 00:05:23+00:00"


@pytest.mark.parametrize('datetime_str',
['2011-11-04T00:05:23Z', '2011-05-07T14:34:14+00:00'])
['2011-11-04T00:05:23Z', '2011-11-04T00:05:23+00:00'])
def test_create_data_row_with_metadata(dataset, image_url, datetime_str):
client = dataset.client
assert len(list(dataset.data_rows())) == 0
Expand All @@ -475,5 +475,4 @@ def test_create_data_row_with_metadata(dataset, image_url, datetime_str):
metadata_fields=metadata_fields)

retrieved_data_row = client.get_data_row(data_row.uid)
assert retrieved_data_row.metadata[0].value == datetime.fromisoformat(
datetime_str)
assert f"{retrieved_data_row.metadata[0].value}" == "2011-11-04 00:05:23+00:00"
13 changes: 13 additions & 0 deletions tests/unit/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import pytest
from labelbox.utils import format_iso_datetime, format_iso_from_string


@pytest.mark.parametrize('datetime_str, expected_datetime_str',
[('2011-11-04T00:05:23Z', '2011-11-04T00:05:23Z'),
('2011-11-04T00:05:23+00:00', '2011-11-04T00:05:23Z'),
('2011-11-04T00:05:23+05:00', '2011-11-03T19:05:23Z'),
('2011-11-04T00:05:23', '2011-11-04T00:05:23Z')])
def test_datetime_parsing(datetime_str, expected_datetime_str):
# NOTE I would normally not take 'expected' using another function from sdk code, but in this case this is exactly the usage in _validate_parse_datetime
assert format_iso_datetime(
format_iso_from_string(datetime_str)) == expected_datetime_str
2 changes: 1 addition & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# content of: tox.ini , put in same dir as setup.py
[tox]
envlist = py36, py37, py38
envlist = py37, py38, py39

[testenv]
# install pytest in the virtualenv where commands will be executed
Expand Down