Add functionality to parse datetimes according C standard format codes. by bendichter · Pull Request #165 · r1chardj0n3s/parse

bendichter · 2023-11-09T03:17:36Z

fix #150

wimglenn · 2023-11-09T03:37:09Z

No f-string please

bendichter · 2023-11-09T03:43:53Z

@wimglenn done.

wimglenn · 2023-11-09T04:00:46Z

Thanks. I just blackened the code in #166 which made some conflicts for you, sorry about that

# Conflicts: # parse.py

bendichter · 2023-11-09T04:40:01Z

@wimglenn fixed

bendichter · 2023-11-21T13:02:11Z

@wimglenn is merging this on the roadmap? Are there any more changes you would like to request?

wimglenn · 2023-11-21T17:18:51Z

@bendichter This looks like a great idea to me, I'll give a careful review when I have a spare moment.

bendichter · 2023-11-21T17:27:14Z

Sounds good. I'd be happy to answer any questions

wimglenn · 2023-11-21T19:28:32Z

I do have a question - I'd really like anything that works with strftime to work here, e.g. this should be able to roundtrip:

>>> dt = datetime.now()
>>> f"{dt:%Y-%m-%d %H:%M:%S}"
'2023-11-21 13:23:27'
>>> parse.parse("{dt:%Y-%m-%d %H:%M:%S}", "2023-11-21 13:23:27")
...
# ValueError: too many values to unpack (expected 2)

So on line 616 where there is name, format = field.split(":") I think it should be name, format = field.split(":", 1). Does that sound reasonable to you?

bendichter · 2023-11-21T19:31:03Z

Yes, good catch! I'll make that change and add a test

wimglenn · 2023-11-21T19:31:59Z

parse.py

+        "Y": "[0-9]{4}",
+        "H": "[0-9]{1,2}",
+        "B": "(?:January|February|March|April|May|June|July|August|September|October|November|December)",
+        "b": "(?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)",


I wonder if these should be made locale-aware...

yeah good idea I'll look into it

Can be addressed at same time as #1

README.rst

bendichter · 2023-11-21T20:21:34Z

I gave it more flexibility for the %z directive. %Z gets complicated

strptime() only accepts certain values for %Z:
any value in time.tzname for your machine’s locale
the hard-coded values UTC and GMT

(source)

If using a timezone out of that range, strptime will fail, even if the timezone is valid:

datetime.strptime("2023-11-21 13:23:27 PST", "%Y-%m-%d %H:%M:%S %Z")

ValueError: time data '2023-11-21 13:23:27 PST' does not match format '%Y-%m-%d %H:%M:%S %Z'

If using a timezone within that range, strptime will succeed however the output datetime is not timezone-aware!

datetime.strptime("2023-11-21 13:23:27 EST", "%Y-%m-%d %H:%M:%S %Z")

datetime.datetime(2023, 11, 21, 13, 23, 27)

This seems to me like a bug. And in any case things are getting messy so I'd rather not support this.

wimglenn · 2023-11-21T22:40:43Z

Yes the zone stuff looks tricky due to Python itself (e.g. https://bugs.python.org/issue22377)
I suppose users can always plug in a dateutil.parser if they want any better than strptime is capable of..

codecov-commenter · 2023-11-22T04:00:33Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.

Project coverage is 94.59%. Comparing base (29e5f8e) to head (58c998b).
Report is 24 commits behind head on master.

Files	Patch %	Lines
parse.py	94.11%	1 Missing and 2 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
- Coverage   94.72%   94.59%   -0.13%     
==========================================
  Files           1        1              
  Lines         512      537      +25     
  Branches      123      131       +8     
==========================================
+ Hits          485      508      +23     
- Misses         17       18       +1     
- Partials       10       11       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

wimglenn · 2023-11-22T04:14:50Z

parse.py



+dt_format_to_regex = {symbol: "[0-9]{2}" for symbol in "ymdIMSUW"}
+dt_format_to_regex.update({"-" + symbol: "[0-9]{1,2}" for symbol in "ymdIMS"})


For some reason I get a problem with these:

>>> datetime(2023, 1, 1).strftime("%Y/%-m/%-d") '2023/1/1'

But

>>> parse("{:%Y/%-m/%-d}", "2023/1/1") ValueError: '-' is a bad directive in format '%Y/%-m/%-d'

Hmmm I'll take a look tomorrow

This is now fixed and tested. It turns out strptime does not use the negative sign (I'm not quite sure why I thought it did). In strftime, %d outputs a zero-padded number e.g. "01". For strptime, %d matches a zero-padded number and can also match a non-zero padded number e.g. "1".

Now the following works:

>>> parse("{:%Y/%m/%d}", "2023/01/01") <Result (datetime.datetime(2023, 1, 1, 0, 0),) {}> >>> parse("{:%Y/%m/%d}", "2023/1/1") <Result (datetime.datetime(2023, 1, 1, 0, 0),) {}>

That's better. %j should get the same treatment, because it has the same issue (i.e. "%-j" doesn't work with strptime, and %j doesn't care if the number is zero-padded or not).

After removing the mapping for "-j", you can also remove the re.escape since the remaining characters are all letters and don't need escaping.

…_flexible_dates # Conflicts: # parse.py

bendichter · 2023-11-23T02:48:24Z

hold it, I think you want j to map to {1,3}. Let's also add a test for it

bendichter · 2023-11-23T03:30:01Z

I tried to see if I could replace some of the existing datetime logic, but most of them allow for variants of each format, e.g. using either slashes or dashes, so this approach does not work well. I was able to change "tc" to use this approach though.

…_flexible_dates # Conflicts: # parse.py

wimglenn · 2023-11-23T03:40:55Z

Let's not change those, because there is little benefit, and I can't be sure it doesn't modify existing behaviour.

I'm also not sure about the condition for returning a datetime vs a time. I think we should either remove that, and always return a datetime, or extend it to return a datetime, time, or date (depending on whether the directives are solely time-related or solely date-related).

bendichter · 2023-11-23T03:50:57Z

OK, I reverted to the old logic for "tc".

The datetime vs. date vs. time thing is tricky. I would like to avoid situations like having parse("{:%H:%M}", "10:26") resolve to datetime(1900, 1, 1, 10, 26, 0). There is precedent in this library for resolving missing year to the current year:

parse/parse.py

Line 194 in 04314d8

y = datetime.today().year

but I still think that only makes sense when a month is present as it is in

parse/tests/test_parse.py

Lines 575 to 589 in 04314d8

    
           y( 
        
               "a {:ts} b", 
        
               "a Nov 21 10:21:36 b", 
        
               datetime(datetime.today().year, 11, 21, 10, 21, 36), 
        
           ) 
        
           y( 
        
               "a {:ts} b", 
        
               "a Nov  1 10:21:36 b", 
        
               datetime(datetime.today().year, 11, 1, 10, 21, 36), 
        
           ) 
        
           y( 
        
               "a {:ts} b", 
        
               "a Nov  1 03:21:36 b", 
        
               datetime(datetime.today().year, 11, 1, 3, 21, 36), 
        
           )

I suppose the logic could be:

If it contains day, month, or year, it's a date. Missing years default to the current year.
If it contains hours, minutes, seconds, or milliseconds, its a time.
If it is a date and a time it's a datetime.

bendichter · 2023-11-23T03:54:02Z

Maybe consider configuring black as a precommit hook, e.g. https://github.com/catalystneuro/neuroconv/blob/1d3d58d4b53570d20b5e81abde7ada7f9f02fa47/.pre-commit-config.yaml#L8-L12

wimglenn · 2023-11-23T04:00:00Z

Yes, that logic sounds good to me.
Directives indicative of dates: aAwdbBmyYjUW
Directives indicative of times: HIpMSfz
I've gotta sign off now so I'll leave it in your capable hands and check it out tomorrow

If it contains day, month, or year, it's a date. Missing years default to the current year. If it contains hours, minutes, seconds, or milliseconds, its a time. If it is a date and a time it's a datetime.

…_flexible_dates

bendichter · 2023-11-24T19:36:24Z

@wimglenn Thanks for all the collaboration on this. I am happy with its current state. Feel free to merge when you are ready.

wimglenn · 2023-11-25T03:04:45Z

Merged and released 1.20.0 to PyPI. Thank you for your contributions, Ben!

wimglenn · 2024-11-15T16:55:16Z

parse.py

+    elif is_time:
+        return dt.time()
+    else:
+        ValueError("Datetime not a date nor a time?")


@bendichter Somehow we both missed a raise here (ref #196)

oops. Thanks for catching that

Add functionality to parse datetimes according C standard format codes.

8237019

bendichter mentioned this pull request Nov 9, 2023

parse arbitrary datetime strings #152

Closed

use regex to dynamically create specific datetime regex paterns

f4b0cbe

change from f-string to str.format syntax

c1bee76

remove copy import

28b6336

Merge branch 'og_master' into parse_flexible_dates

30173ed

# Conflicts: # parse.py

wimglenn reviewed Nov 21, 2023

View reviewed changes

allow for colons in time format

cd31fc2

wimglenn reviewed Nov 21, 2023

View reviewed changes

README.rst Outdated Show resolved Hide resolved

allow for more flexible parsing of %z

3d2ea99

shield Python 2 from timezone features

32d83fb

bendichter and others added 2 commits November 21, 2023 17:55

add time parsing

acdfeb0

uglify code with black

5cc7111

bump version

f71aa7a

wimglenn reviewed Nov 22, 2023

View reviewed changes

bendichter and others added 3 commits November 22, 2023 13:26

handle and test single digits for day and month

72591f7

Merge remote-tracking branch 'origin/parse_flexible_dates' into parse…

c3a4f32

…_flexible_dates # Conflicts: # parse.py

remove %-j handling

5210e18

bendichter and others added 5 commits November 22, 2023 21:51

make j flexible number of digits

fb6d2c0

simplify and reorder, to match docs

550f5e5

just use the map directly

8771d3f

readability improvements

fb0c8d9

change "tc" to use generic datetime parsing approach

77328ef

bendichter added 2 commits November 22, 2023 22:30

Merge remote-tracking branch 'origin/parse_flexible_dates' into parse…

11342e1

…_flexible_dates # Conflicts: # parse.py

use new conv variable

34edfc8

bendichter and others added 3 commits November 22, 2023 22:43

revert to old logic for tc

2c5f905

blacken again

6af64dd

blacken again sorry

b08ed21

bendichter and others added 3 commits November 22, 2023 23:15

new logic:

4b1ba61

If it contains day, month, or year, it's a date. Missing years default to the current year. If it contains hours, minutes, seconds, or milliseconds, its a time. If it is a date and a time it's a datetime.

Merge remote-tracking branch 'origin/parse_flexible_dates' into parse…

fd1a414

…_flexible_dates

doc update

44c97bb

test roundtrip every directive

58c998b

wimglenn merged commit a2e1334 into r1chardj0n3s:master Nov 25, 2023

bendichter mentioned this pull request Nov 25, 2023

change from using fparse to using parse catalystneuro/neuroconv#654

Merged

wimglenn reviewed Nov 15, 2024

View reviewed changes



		dt_format_to_regex = {symbol: "[0-9]{2}" for symbol in "ymdIMSUW"}
		dt_format_to_regex.update({"-" + symbol: "[0-9]{1,2}" for symbol in "ymdIMS"})

Conversation

bendichter commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wimglenn commented Nov 9, 2023

Uh oh!

bendichter commented Nov 9, 2023

Uh oh!

wimglenn commented Nov 9, 2023

Uh oh!

bendichter commented Nov 9, 2023

Uh oh!

bendichter commented Nov 21, 2023

Uh oh!

wimglenn commented Nov 21, 2023

Uh oh!

bendichter commented Nov 21, 2023

Uh oh!

wimglenn commented Nov 21, 2023

Uh oh!

bendichter commented Nov 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bendichter commented Nov 21, 2023

Uh oh!

wimglenn commented Nov 21, 2023

Uh oh!

codecov-commenter commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bendichter Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bendichter Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bendichter commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bendichter commented Nov 23, 2023

Uh oh!

wimglenn commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bendichter commented Nov 23, 2023

Uh oh!

bendichter commented Nov 23, 2023

Uh oh!

wimglenn commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bendichter commented Nov 24, 2023

Uh oh!

wimglenn commented Nov 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

bendichter commented Nov 9, 2023 •

edited

Loading

codecov-commenter commented Nov 22, 2023 •

edited

Loading

bendichter Nov 22, 2023 •

edited

Loading

bendichter Nov 22, 2023 •

edited

Loading

bendichter commented Nov 23, 2023 •

edited

Loading

wimglenn commented Nov 23, 2023 •

edited

Loading

wimglenn commented Nov 23, 2023 •

edited

Loading