used regular expression in `format_is_iso()` #50468

ShashwatAgrawal20 · 2022-12-28T15:17:24Z

closes PERF use regular expression in format_is_iso? #50465 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/v2.0.0.rst file if fixing a bug or adding a new feature.

Added regular expressions in the format_is_iso() function

def format_is_iso(f: str) -> bint:
    """
    Does format match the iso8601 set that can be handled by the C parser?
    Generally of form YYYY-MM-DDTHH:MM:SS - date separator can be different
    but must be consistent.  Leading 0s in dates and times are optional.
    """
    iso_regex = \
        r"^\d{4}(-\d{2}(-\d{2}(T\d{2}:\d{2}:\d{2}(\.\d+)?(([+-]\d{2}:\d{2})|Z)?)?)?)?$"
    excluded_formats = ["%Y%m%d", "%Y%m", "%Y"]
    if re.match(iso_regex, f) and f not in excluded_formats:
        return True
    return False

MarcoGorelli · 2022-12-28T15:24:32Z

pandas/_libs/tslibs/parsing.pyx

+    iso_regex = \
+        r"^\d{4}(-\d{2}(-\d{2}(T\d{2}:\d{2}:\d{2}(\.\d+)?(([+-]\d{2}:\d{2})|Z)?)?)?)?$"


thanks for working on this - to keep it readable, how about making this a verbose with comments?

see https://stackoverflow.com/a/13852096/4451315

@MarcoGorelli added some comments for explaining the regular expression and how it's evaluating

MarcoGorelli

Please check the link I provided, it explains how to write a regular expression with the verbose flag

Did you try running this locally?

ShashwatAgrawal20 · 2022-12-28T17:13:18Z

Oh sorry didn't read the lin, let me fix it

MarcoGorelli · 2022-12-28T17:48:52Z

pandas/_libs/tslibs/parsing.pyx

+        r"""
+        ^                       # start of string
+        \d{4}                   # match a 4-digit year
+        (-\d{2})?               # optionally match a 2-digit month


date_sep isn't limited to -

ShashwatAgrawal20 · 2022-12-28T17:50:26Z

import re

def format_is_iso(f: str) -> bool:
    """
    Does format match the iso8601 set that can be handled by the C parser?
    Generally of form YYYY-MM-DDTHH:MM:SS - date separator can be different
    but must be consistent.  Leading 0s in dates and times are optional.
    """
    iso_regex = re.compile(
        r"""
        ^                         # start of string
        \d{4}                     # match a 4-digit year
        (-\d{2})?                 # optionally match a 2-digit month
        (-\d{2})?                 # optionally match a 2-digit day
        (T\d{2}:\d{2}:\d{2}       # match time in the format "THH:MM:SS"
        (\.\d+)?                  # optionally match a decimal and fractional seconds
        ([+-]\d{2}:\d{2}|Z)?)?    # optionally match a timezone in the format "+HH:MM" or "Z"
        $                         # end of string
        """,
        re.VERBOSE,
    )
    excluded_formats = ["%Y%m%d", "%Y%m", "%Y"]
    if re.match(iso_regex, f) and f not in excluded_formats:
        return True
    return False


res = format_is_iso('2022-12-38T17:26:49Z')
print(res)

ShashwatAgrawal20 · 2022-12-28T18:43:08Z

        ([ -/\\.]\d{2}|\d{2})?  # optionally match a 2-digit month
        ([ -/\\.]\d{2}|\d{2})?  # optionally match a 2-digit day
        ([ T]\d{2}:\d{2}:\d{2}  # match time in the format "THH:MM:SS"

Added some separators regex to separate the dates & time

MarcoGorelli · 2022-12-28T22:13:44Z

getting there, but it's not quite right,it's failing lots of tests

MarcoGorelli

tests are still failing

at a minimum, you need to get

pytest pandas/tests/tslibs/test_parsing.py

to pass - please make sure that this passes locally for you

ShashwatAgrawal20 · 2022-12-29T12:40:37Z

tests are still failing

at a minimum, you need to get
pytest pandas/tests/tslibs/test_parsing.py
to pass - please make sure that this passes locally for you

Sorry about the inconvenience I didn't knew we can test this on local system

ShashwatAgrawal20 · 2022-12-29T12:47:37Z

@MarcoGorelli ran the test on the non-regular-expression version they are also failing. What should I do? any suggestions?

MarcoGorelli · 2022-12-29T13:25:23Z

did you compile the C extensions? https://pandas.pydata.org/docs/dev/development/contributing_environment.html#step-3-build-and-install-pandas

ShashwatAgrawal20 · 2022-12-29T15:32:51Z

@MarcoGorelli ran the test on the non-regular-expression version they are also failing. What should I do? any suggestions?

Hey, @MarcoGorelli didn't understand why it's assert False == True in all. It should compare the expected value with the result value.
As in the file pandas/tests/tslibs/test_parsing.py

@pytest.mark.parametrize(
    "fmt,expected",
    [
        ("%Y %m %d %H:%M:%S", True),
        ("%Y/%m/%d %H:%M:%S", True),
        (r"%Y\%m\%d %H:%M:%S", True),
        ("%Y-%m-%d %H:%M:%S", True),
        ("%Y.%m.%d %H:%M:%S", True),
        ("%Y%m%d %H:%M:%S", True),
        ("%Y-%m-%dT%H:%M:%S", True),
        ("%Y-%m-%dT%H:%M:%S%z", True),
        ("%Y-%m-%dT%H:%M:%S%Z", False),
        ("%Y-%m-%dT%H:%M:%S.%f", True),
        ("%Y-%m-%dT%H:%M:%S.%f%z", True),
        ("%Y-%m-%dT%H:%M:%S.%f%Z", False),
        ("%Y%m%d", False),
        ("%Y%m", False),
        ("%Y", False),
        ("%Y-%m-%d", True),
        ("%Y-%m", True),
    ],
)
def test_is_iso_format(fmt, expected):
    # see gh-41047
    result = parsing.format_is_iso(fmt)
    assert result == expected

Wrote the code with the dates in the format of the above and checked the result with the excepted output, the result output is matching with the expected out

import re


def format_is_iso(f: str) -> bool:
    """
    Does format match the iso8601 set that can be handled by the C parser?
    Generally of form YYYY-MM-DDTHH:MM:SS - date separator can be different
    but must be consistent.  Leading 0s in dates and times are optional.
    """
    iso_regex = re.compile(
        r"""
        ^                       # start of string
        \d{4}                   # match a 4-digit year
        ([ -/\\.]\d{2}|\d{2})?  # optionally match a 2-digit month
        ([ -/\\.]\d{2}|\d{2})?  # optionally match a 2-digit day
        ([ T]\d{2}:\d{2}:\d{2}  # match time in the format "THH:MM:SS"
        (\.\d+)?                # match a decimal and fractional seconds
        ([+-]\d{2}:\d{2}|Z)?)?  # match timezone in the format "+HH:MM" or "Z"
        $                       # end of string
        """,
        re.VERBOSE,
    )

    excluded_formats = [
        r"^\d{4}\d{2}\d{2}$",  # %Y%m%d
        r"^\d{4}\d{2}$",       # %Y%m
        r"^\d{4}$",            # %Y
    ]

    if any(re.match(pattern, f) for pattern in excluded_formats):
        return False

    return bool(re.match(iso_regex, f))


dates = [
    "2022 12 29 12:34:56",
    "2022/12/29 12:34:56",
    "2022\\12\\29 12:34:56",
    "2022-12-29 12:34:56",
    "2022.12.29 12:34:56",
    "20221229 12:34:56",
    "2022-12-29T12:34:56",
    "2022-12-29T12:34:56+01:00",
    "2022-12-29T12:34:56CET",
    "2022-12-29T12:34:56.123456",
    "2022-12-29T12:34:56.123456+01:00",
    "2022-12-29T12:34:56.123456CET",
    "20221229",
    "202212",
    "2022",
    "2022-12-29",
    "2022-12"
]

for date in dates:
    res = format_is_iso(date)
    print(date, res)

But at the time of pytest it fails again

MarcoGorelli · 2022-12-29T16:37:37Z

does the test pass on the main branch? if not, then I guess that your pandas build isn't quite correct - check the installation instructions I sent earlier

ShashwatAgrawal20 · 2022-12-29T16:53:37Z

does the test pass on the main branch? if not, then I guess that your pandas build isn't quite correct - check the installation instructions I sent earlier

The test got passed on the main branch with the non-regular-expression.

MarcoGorelli · 2022-12-29T17:02:24Z

then you need to fix up your regular expression 😄

MarcoGorelli · 2022-12-29T17:03:04Z

this might help https://stackoverflow.com/a/74953592/4451315

ShashwatAgrawal20 · 2022-12-30T03:12:58Z

Hey the upstream got the function remove, don't know what exactly happened

MarcoGorelli · 2022-12-30T08:07:59Z

it moved to a different file, if you do a search (e.g. with git grep) you'll find it

…50465

MarcoGorelli · 2023-01-01T15:23:49Z

Nice! If that works, it looks good to me!

MarcoGorelli

If this regex works, it looks good to me, thanks for sticking with it

Would be good to time it too

MarcoGorelli · 2023-01-01T15:33:13Z

pandas/_libs/tslibs/strptime.pyx

+        ^                     # start of string
+        %Y                    # Year
+        (?:([-/ \\.]?)%m      # month with or without separators
+        (?:\1%d               # day with or without separators


perhaps let's update the comment to "day with same separator as for year-month"?

MarcoGorelli · 2023-01-01T15:34:41Z

pandas/_libs/tslibs/strptime.pyx

+    excluded_formats = [
+        r"^%Y%m$",
+    ]
+    if any(re.match(pattern, f) for pattern in excluded_formats):
+        return False
+    return bool(re.match(iso_regex, f))


I don't think we need a regex for the excluded formats - would

return re.match(iso_regex, f) is not None and f not in excluded_formats

work?

yea worked fine without the regex excluded_format

MarcoGorelli · 2023-01-01T15:35:05Z

pandas/_libs/tslibs/strptime.pyx

 from pandas._libs.tslibs.np_datetime import OutOfBoundsDatetime
+


could you revert these unrelated changes please?

ShashwatAgrawal20 · 2023-01-01T15:36:45Z

Nice! If that works, it looks good to me!

hey, by the way, pytest pandas/tests/tslibs/test_parsing.py having some import error, is that on my device or it is actually an issue?

MarcoGorelli · 2023-01-01T15:40:12Z

it works for me, so my only guess is that it has to do with C extensions again - if you post your error (here or on slack) I'll take a look

ShashwatAgrawal20 · 2023-01-01T15:48:56Z

it works for me, so my only guess is that it has to do with C extensions again - if you post your error (here or on slack) I'll take a look

Sorry for the inconvenience still a noob in the open source world, but learned a lot by your guidance

MarcoGorelli · 2023-01-01T16:06:42Z

no worries - I suspect it's this #47305
I'd suggest cleaning and rebuilding:

git clean -fxfd pandas
python setup.py build_ext -j 4

MarcoGorelli

Cool, seems to be working!

Couple of minor comments - then I'll time this and we'll see if we can get it in

pandas/_libs/tslibs/strptime.pyx

MarcoGorelli · 2023-01-01T18:05:20Z

Timings look fine:

upstream/main:

In [2]: %%timeit
   ...: format_is_iso('%Y-%m-%d %H:%M%S%z')
   ...: 
   ...: 
2.64 µs ± 27.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

here:

In [1]:  from pandas._libs.tslibs.strptime import format_is_iso

In [2]: %%timeit
   ...: format_is_iso('%Y-%m-%d %H:%M%S%z')
   ...: 
   ...: 
1.39 µs ± 67.7 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

(note: I had to change cdef to cpdef in both cases to be able to run this test)

I prefer it written like this anyway

MarcoGorelli

Looks good to me - @WillAyd seeing as you'd suggested this, any thoughts?

pandas/_libs/tslibs/strptime.pyx

WillAyd · 2023-01-01T22:05:20Z

lgtm. Nice performance boost. Anything else left to do or want to take out of draft status?

MarcoGorelli · 2023-01-02T10:36:01Z

@ShashwatAgrawal20 if this is ready, could you mark the PR as ready for review please? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#marking-a-pull-request-as-ready-for-review

let's also add a whatsnew note, something like

Performance improvement in :func:`to_datetime` when format is given or can be inferred (:issue:`50465`)

ShashwatAgrawal20 · 2023-01-02T11:24:11Z

@ShashwatAgrawal20 if this is ready, could you mark the PR as ready for review please? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#marking-a-pull-request-as-ready-for-review

let's also add a whatsnew note, something like
Performance improvement in :func:`to_datetime` when format is given or can be inferred (:issue:`50465`)

doc/source/whatsnew/v2.0.0.rst here ?

MarcoGorelli · 2023-01-02T13:03:49Z

That's right! This is better than the original btw, because this would fail on'%Y-%m-%d %H:', whereas the original would have accepted it

Leaving open a bit in case there's objections, then will merge

ShashwatAgrawal20 · 2023-01-02T15:43:52Z

That's right! This is better than the original btw, because this would fail on'%Y-%m-%d %H:', whereas the original would have accepted it

Leaving open a bit in case there's objections, then will merge

wouldn't have been possible without your guidance.

MarcoGorelli

thanks @ShashwatAgrawal20

MarcoGorelli reviewed Dec 28, 2022

View reviewed changes

ShashwatAgrawal20 requested a review from MarcoGorelli December 28, 2022 16:07

MarcoGorelli suggested changes Dec 28, 2022

View reviewed changes

ShashwatAgrawal20 force-pushed the regular-expression-50465 branch from 38bd5f0 to 77a8f02 Compare December 28, 2022 17:46

MarcoGorelli suggested changes Dec 28, 2022

View reviewed changes

Shashwat added 5 commits December 29, 2022 00:09

performace: used regular expression 50465

82b7505

added comments for the code

67d5612

fixed broken comment

9b13481

verbose regex

23679e4

added more seperators

f30c0c2

ShashwatAgrawal20 force-pushed the regular-expression-50465 branch from 2dfa309 to f30c0c2 Compare December 28, 2022 18:40

ShashwatAgrawal20 requested a review from MarcoGorelli December 28, 2022 18:44

Merge branch 'main' into regular-expression-50465

0d0d708

MarcoGorelli suggested changes Dec 29, 2022

View reviewed changes

working excluding format

6070a00

Merge remote-tracking branch 'upstream/main' into regular-expression-…

3b5aa1d

…50465

modified version

5561743

MarcoGorelli suggested changes Jan 1, 2023

View reviewed changes

non regex excluded format

42bc699

MarcoGorelli suggested changes Jan 1, 2023

View reviewed changes

pandas/_libs/tslibs/strptime.pyx Outdated Show resolved Hide resolved

pandas/_libs/tslibs/strptime.pyx Outdated Show resolved Hide resolved

MarcoGorelli added 2 commits January 1, 2023 18:05

Update pandas/_libs/tslibs/strptime.pyx

a22783e

Update pandas/_libs/tslibs/strptime.pyx

8ba5d6b

MarcoGorelli approved these changes Jan 1, 2023

View reviewed changes

MarcoGorelli reviewed Jan 1, 2023

View reviewed changes

pandas/_libs/tslibs/strptime.pyx Outdated Show resolved Hide resolved

Update pandas/_libs/tslibs/strptime.pyx

287a1b9

Merge branch 'main' into regular-expression-50465

7cd2cc1

ShashwatAgrawal20 marked this pull request as ready for review January 2, 2023 11:16

added whatsnew

f3d3b3d

ShashwatAgrawal20 requested a review from MarcoGorelli January 2, 2023 12:20

MarcoGorelli added this to the 2.0 milestone Jan 2, 2023

MarcoGorelli added Datetime Datetime data dtype Performance Memory or execution speed performance labels Jan 2, 2023

MarcoGorelli approved these changes Jan 2, 2023

View reviewed changes

MarcoGorelli merged commit 99859e4 into pandas-dev:main Jan 2, 2023

ShashwatAgrawal20 deleted the regular-expression-50465 branch May 11, 2024 10:17

		iso_regex = \
		r"^\d{4}(-\d{2}(-\d{2}(T\d{2}:\d{2}:\d{2}(\.\d+)?(([+-]\d{2}:\d{2})\|Z)?)?)?)?$"

		from pandas._libs.tslibs.np_datetime import OutOfBoundsDatetime

used regular expression in format_is_iso() #50468

used regular expression in format_is_iso() #50468

Conversation

ShashwatAgrawal20 commented Dec 28, 2022 • edited Loading

MarcoGorelli Dec 28, 2022

Choose a reason for hiding this comment

ShashwatAgrawal20 Dec 28, 2022

Choose a reason for hiding this comment

MarcoGorelli left a comment

Choose a reason for hiding this comment

ShashwatAgrawal20 commented Dec 28, 2022

MarcoGorelli Dec 28, 2022

Choose a reason for hiding this comment

ShashwatAgrawal20 commented Dec 28, 2022

ShashwatAgrawal20 commented Dec 28, 2022

MarcoGorelli commented Dec 28, 2022

MarcoGorelli left a comment • edited Loading

Choose a reason for hiding this comment

ShashwatAgrawal20 commented Dec 29, 2022

ShashwatAgrawal20 commented Dec 29, 2022

MarcoGorelli commented Dec 29, 2022

ShashwatAgrawal20 commented Dec 29, 2022

MarcoGorelli commented Dec 29, 2022

ShashwatAgrawal20 commented Dec 29, 2022

MarcoGorelli commented Dec 29, 2022

MarcoGorelli commented Dec 29, 2022

ShashwatAgrawal20 commented Dec 30, 2022 • edited Loading

MarcoGorelli commented Dec 30, 2022

MarcoGorelli commented Jan 1, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli Jan 1, 2023

Choose a reason for hiding this comment

MarcoGorelli Jan 1, 2023

Choose a reason for hiding this comment

ShashwatAgrawal20 Jan 1, 2023

Choose a reason for hiding this comment

MarcoGorelli Jan 1, 2023

Choose a reason for hiding this comment

ShashwatAgrawal20 commented Jan 1, 2023

MarcoGorelli commented Jan 1, 2023

ShashwatAgrawal20 commented Jan 1, 2023

MarcoGorelli commented Jan 1, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli commented Jan 1, 2023 • edited Loading

MarcoGorelli left a comment

Choose a reason for hiding this comment

WillAyd commented Jan 1, 2023

MarcoGorelli commented Jan 2, 2023

ShashwatAgrawal20 commented Jan 2, 2023

MarcoGorelli commented Jan 2, 2023

ShashwatAgrawal20 commented Jan 2, 2023

MarcoGorelli left a comment • edited Loading

Choose a reason for hiding this comment

used regular expression in `format_is_iso()` #50468

used regular expression in `format_is_iso()` #50468

ShashwatAgrawal20 commented Dec 28, 2022 •

edited

Loading

MarcoGorelli left a comment •

edited

Loading

ShashwatAgrawal20 commented Dec 30, 2022 •

edited

Loading

MarcoGorelli commented Jan 1, 2023 •

edited

Loading

MarcoGorelli left a comment •

edited

Loading