From 20da07562aab1d9041170f70e1a3dc086f5c1b90 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 1 Oct 2019 10:53:18 +0200 Subject: Add converter for ISO-formatted datetime strings Related to https://github.com/python-discord/bot/issues/458 This commit adds a converter that automatically parses ISO-formatted datetime strings and returns a `datetime.datetime` object. It uses `dateutil.parser.isoparse` to do the heavy lifting, so it supports the same formats as this method. In addition, I have added tests that ensure that it accepts certain formats and added a description of these 'guaranteed' formats to the `ISODate.convert` docstring. This commit should make it easy to implement #485 --- tests/test_converters.py | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) (limited to 'tests/test_converters.py') diff --git a/tests/test_converters.py b/tests/test_converters.py index 35fc5d88e..aa692f9f8 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -8,6 +8,7 @@ from discord.ext.commands import BadArgument from bot.converters import ( Duration, + ISODateTime, TagContentConverter, TagNameConverter, ValidPythonIdentifier, @@ -184,3 +185,57 @@ def test_duration_converter_for_invalid(duration: str): converter = Duration() with pytest.raises(BadArgument, match=f'`{duration}` is not a valid duration string.'): asyncio.run(converter.convert(None, duration)) + + +@pytest.mark.parametrize( + ("datetime_string", "expected_dt"), + ( + # `YYYY-mm-ddTHH:MM:SS` | `YYYY-mm-dd HH:MM:SS` + ('2019-09-02T02:03:05', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 02:03:05', datetime.datetime(2019, 9, 2, 2, 3, 5)), + + # `YYYY-mm-ddTHH:MM` | `YYYY-mm-dd HH:MM` + ('2019-11-12T09:15', datetime.datetime(2019, 11, 12, 9, 15)), + ('2019-11-12 09:15', datetime.datetime(2019, 11, 12, 9, 15)), + + # `YYYY-mm-dd` + ('2019-04-01', datetime.datetime(2019, 4, 1)), + + # `YYYY-mm` + ('2019-02-01', datetime.datetime(2019, 2, 1)), + + # `YYYY` + ('2025', datetime.datetime(2025, 1, 1)), + ), +) +def test_isodatetime_converter_for_valid(datetime_string: str, expected_dt: datetime.datetime): + converter = ISODateTime() + assert asyncio.run(converter.convert(None, datetime_string)) == expected_dt + + +@pytest.mark.parametrize( + ("datetime_string"), + ( + # Make sure it doesn't interfere with the Duration converation + ('1Y'), + ('1d'), + ('1H'), + + # Check if it fails when only providing the optional time part + ('10:10:10'), + ('10:00'), + + # Invalid date format + ('19-01-01'), + + # Other non-valid strings + ('fisk the tag master'), + ), +) +def test_isodatetime_converter_for_invalid(datetime_string: str): + converter = ISODateTime() + with pytest.raises( + BadArgument, + match=f"`{datetime_string}` is not a valid ISO-8601 datetime string", + ): + asyncio.run(converter.convert(None, datetime_string)) -- cgit v1.2.3 From 5f81b80a4dea49195053ab0177f4fd9aa9bea5e5 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 2 Oct 2019 07:27:06 +0200 Subject: Apply docstring review suggestion Co-Authored-By: Mark --- tests/test_converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/test_converters.py') diff --git a/tests/test_converters.py b/tests/test_converters.py index aa692f9f8..8093f55ac 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -216,7 +216,7 @@ def test_isodatetime_converter_for_valid(datetime_string: str, expected_dt: date @pytest.mark.parametrize( ("datetime_string"), ( - # Make sure it doesn't interfere with the Duration converation + # Make sure it doesn't interfere with the Duration converter ('1Y'), ('1d'), ('1H'), -- cgit v1.2.3 From a8b600217cb9ab4524bb307f0a6a922a0d8815be Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 2 Oct 2019 10:42:43 +0200 Subject: Make ISODateTime return tz-unaware datetime The parser we use, `dateutil.parsers.isoparse` returns a timezone- aware or timezone-unaware `datetime` object depending on whether or not the datetime string included a timezone offset specification. Since we can't compare tz-aware objects to tz-unaware objects it's better to make sure our converter is consistent in the type it will return. For now, I've chosen to return tz-unaware datetime objects, since `discord.py` also returns tz-unaware datetime objects when accessing datetime-related attributes of objects. Since we're likely to compare "our" datetime objects to discord.py-provided datetime objects, I think that's the most parsimonious option for now. Note: It's probably a good idea to open a larger discussion about using timezone-aware datetime objects throughout the library to avoid a UTC-time being interpreted as localtime. This will require a broader discussion than this commit/PR allows, though. --- bot/converters.py | 13 ++++++++++++- tests/test_converters.py | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) (limited to 'tests/test_converters.py') diff --git a/bot/converters.py b/bot/converters.py index 59a6f6b07..27223e632 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -5,6 +5,7 @@ from ssl import CertificateError from typing import Union import dateutil.parser +import dateutil.tz import discord from aiohttp import ClientConnectorError from dateutil.relativedelta import relativedelta @@ -227,12 +228,18 @@ class ISODateTime(Converter): The converter is flexible in the formats it accepts, as it uses the `isoparse` method of `dateutil.parser`. In general, it accepts datetime strings that start with a date, - optionally followed by a time. + optionally followed by a time. Specifying a timezone offset in the datetime string is + supported, but the `datetime` object will be converted to UTC and will be returned without + `tzinfo` as a timezone-unaware `datetime` object. See: https://dateutil.readthedocs.io/en/stable/parser.html#dateutil.parser.isoparse Formats that are guaranteed to be valid by our tests are: + - `YYYY-mm-ddTHH:MM:SSZ` | `YYYY-mm-dd HH:MM:SSZ` + - `YYYY-mm-ddTHH:MM:SS±HH:MM` | `YYYY-mm-dd HH:MM:SS±HH:MM` + - `YYYY-mm-ddTHH:MM:SS±HHMM` | `YYYY-mm-dd HH:MM:SS±HHMM` + - `YYYY-mm-ddTHH:MM:SS±HH` | `YYYY-mm-dd HH:MM:SS±HH` - `YYYY-mm-ddTHH:MM:SS` | `YYYY-mm-dd HH:MM:SS` - `YYYY-mm-ddTHH:MM` | `YYYY-mm-dd HH:MM` - `YYYY-mm-dd` @@ -247,4 +254,8 @@ class ISODateTime(Converter): except ValueError: raise BadArgument(f"`{datetime_string}` is not a valid ISO-8601 datetime string") + if dt.tzinfo: + dt = dt.astimezone(dateutil.tz.UTC) + dt = dt.replace(tzinfo=None) + return dt diff --git a/tests/test_converters.py b/tests/test_converters.py index 8093f55ac..86e8f2249 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -190,6 +190,27 @@ def test_duration_converter_for_invalid(duration: str): @pytest.mark.parametrize( ("datetime_string", "expected_dt"), ( + + # `YYYY-mm-ddTHH:MM:SSZ` | `YYYY-mm-dd HH:MM:SSZ` + ('2019-09-02T02:03:05Z', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 02:03:05Z', datetime.datetime(2019, 9, 2, 2, 3, 5)), + + # `YYYY-mm-ddTHH:MM:SS±HH:MM` | `YYYY-mm-dd HH:MM:SS±HH:MM` + ('2019-09-02T03:18:05+01:15', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 03:18:05+01:15', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02T00:48:05-01:15', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 00:48:05-01:15', datetime.datetime(2019, 9, 2, 2, 3, 5)), + + # `YYYY-mm-ddTHH:MM:SS±HHMM` | `YYYY-mm-dd HH:MM:SS±HHMM` + ('2019-09-02T03:18:05+0115', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 03:18:05+0115', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02T00:48:05-0115', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02 00:48:05-0115', datetime.datetime(2019, 9, 2, 2, 3, 5)), + + # `YYYY-mm-ddTHH:MM:SS±HH` | `YYYY-mm-dd HH:MM:SS±HH` + ('2019-09-02 03:03:05+01', datetime.datetime(2019, 9, 2, 2, 3, 5)), + ('2019-09-02T01:03:05-01', datetime.datetime(2019, 9, 2, 2, 3, 5)), + # `YYYY-mm-ddTHH:MM:SS` | `YYYY-mm-dd HH:MM:SS` ('2019-09-02T02:03:05', datetime.datetime(2019, 9, 2, 2, 3, 5)), ('2019-09-02 02:03:05', datetime.datetime(2019, 9, 2, 2, 3, 5)), -- cgit v1.2.3 From 629cb4d05405a155715da765a2408be9156eb215 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 3 Oct 2019 07:14:31 +0200 Subject: Check if tzinfo is None in ISODateTime test As we have decided that the converter should return naive datetime objects, we should explicitly test that datetime strings with a timezone offset are still converted to a naive datetime object. I have done this by adding a `tzinfo is None` assertion. --- tests/test_converters.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tests/test_converters.py') diff --git a/tests/test_converters.py b/tests/test_converters.py index 86e8f2249..f69995ec6 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -231,7 +231,9 @@ def test_duration_converter_for_invalid(duration: str): ) def test_isodatetime_converter_for_valid(datetime_string: str, expected_dt: datetime.datetime): converter = ISODateTime() - assert asyncio.run(converter.convert(None, datetime_string)) == expected_dt + converted_dt = asyncio.run(converter.convert(None, datetime_string)) + assert converted_dt.tzinfo is None + assert converted_dt == expected_dt @pytest.mark.parametrize( -- cgit v1.2.3