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

feat(strptime):handle %b and %B directive. #121

Merged
merged 5 commits into from Feb 16, 2022

Conversation

Mojtaba-saf
Copy link
Contributor

@Mojtaba-saf Mojtaba-saf commented Feb 15, 2022

in response to this issue
as I discussed there If patterns are changed to use '\d' instead of [0-9] regex patterns for python 3 can detect Unicode numbers.

to detect month names in the string we can use these patterns.

    '%B': '(?P<B>[a-zA-Z\u0600-\u06EF\uFB8A\u067E\u0686\u06AF]{3,12})'
    '%b': '(?P<b>[a-zA-Z]{3})',

these patterns detect month names.
I tested it against these examples and it works fine.

jdatetime.datetime.strptime('14 Ordibehesht 1400', '%d %B %Y')
jdatetime.datetime.strptime('14 ordibehesht 1400', '%d %B %Y')
jdatetime.datetime.strptime('۱۴ Ordibehesht ۱۴۰۰', '%d %B %Y')
jdatetime.datetime.strptime('۱۴ ordibehesht ۱۴۰۰', '%d %B %Y')
jdatetime.datetime.strptime('1۴ Ordibehesht 14۰۰', '%d %B %Y')
jdatetime.datetime.strptime('۱4 ordibehesht 14۰0', '%d %B %Y')
jdatetime.datetime.strptime('۱۴ اردیبهشت ۱۴۰۰', '%d %B %Y')
jdatetime.datetime.strptime('14 اردیبهشت 1400', '%d %B %Y')
jdatetime.datetime.strptime('1۴ اردیبهشت ۱4۰0', '%d %B %Y')
jdatetime.datetime.strptime('14 Ord 1400', '%d %b %Y')
jdatetime.datetime.strptime('۱۴ Ord ۱۴۰۰', '%d %b %Y')
jdatetime.datetime.strptime('۱4 Ord 14۰0', '%d %b %Y')

@hramezani
Copy link
Collaborator

Thanks @Mojtaba-saf, Please add some tests for this path.
You can add your tested example as test cases

tests/test_jdatetime.py Outdated Show resolved Hide resolved
@hramezani
Copy link
Collaborator

Let's add another test for invalid month name:

    def test_strptime_invalid_date_string_b_B_directive(self):
        with self.assertRaises(
            ValueError,
            msg="time data '14 foo 1400' does not match format '%d %b %Y'"
        ):
            jdatetime.datetime.strptime('14 foo 1400', '%d %b %Y')

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>
@Mojtaba-saf
Copy link
Contributor Author

sorry I'm not familiar with writing test cases. should this test for invalid month name be exactly like what you said or should it be like the last one and contain multiple subtests?
I ask that because I think it should at least be 2 test cases one for B and one for b directive.

@hramezani
Copy link
Collaborator

sorry I'm not familiar with writing test cases. should this test for invalid month name be exactly like what you said or should it be like the last one and contain multiple subtests?

I would add it to a separate test function.

I ask that because I think it should at least be 2 test cases one for B and one for b directive.

Yes, you can another test case for B as well. like:

    def test_strptime_b_B_directive_invalid_month(self):
        tests = (
            ('۱4 foo 14۰0', '%d %B %Y'),
            ('۱4 bar 14۰0', '%d %b %Y'),
        )
        for date_string, date_format in tests:
            with self.subTest(date_string=date_string, date_format=date_format):
                with self.assertRaises(
                    ValueError,
                    msg=f"time data '{date_string}' does not match format '{date_format}'"
                ):
                    jdatetime.datetime.strptime(date_string, date_format)

Copy link
Collaborator

@hramezani hramezani left a comment

Choose a reason for hiding this comment

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

Thanks @Mojtaba-saf 👍

@hramezani hramezani merged commit 03eb84e into slashmili:main Feb 16, 2022
@slashmili
Copy link
Owner

Thanks @Mojtaba-saf ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants