Consider local time formats when reading runtimes

[ClePol]

I had a crash where my time format in the recon-surf was 'Di 16. Mai 17:36:34 CEST 2023', which is localized to german.

I'm not sure why that's the case, but it wouldn't hurt to have a fallback to try to read the local timeformat. The function dateutil.parser.parse does not consider local timeformats, but datetime.datetime.strptime does.

Edit: This in not a problem in docker, but when running the recon-all pipeline locally. With this fix in the stable branch the pipeline runs locally without error.

[LeHenschel]

Looks fine with me. Before merging, I would suggest that @AhmedFaisal95 makes sure that this does not break anything inside the time extraction scripts.

[af-a]

Hi @ClePol. Thanks for the suggestion.

The locale check makes sense, but we can't use datetime.datetime.strptime because it fails in some cases where the recon-surf date-time string contains an extra empty space in a single-digit day sub-string, e.g. "#@# Intensity Normalization2 Tue Jan 3 06:30:17 UTC 2023" (instead of "#@# Intensity Normalization2 Tue Jan 17 HH:MM:SS UTC 2023").

This is why we switched from datetime.datetime.strptime to dateutil.parser.parse, which is more robust can handle this case, a while ago (see 073d019).
Note: At the time, we decided to only support the standard English datetime format in favour of the more robust parser.

I suggest that we find a different solution that still considers the local locale settings.

Let me know if you have any ideas.

[ClePol]

Hi, thanks for taking a look. I implemented this as a fallback to use when the current method fails. So to me it seems like an absolute improvement to try a second function. A straightforward fix for your failure case would be to detect the single digit day substring and prepend a zero. Alternatively there could be a second fallback. Unfortunately I don't have time to test this right now, but I'll update if I run into this again during local testing.

…

On Tue, Jun 13, 2023, 10:09 Ahmed Faisal Abdelrahman < ***@***.***> wrote: Hi @ClePol <https://github.com/ClePol>. Thanks for the suggestion. The locale check makes sense, but we can't use datetime.datetime.strptime because it fails in some cases where the recon-surf date-time string contains an extra empty space in a single-digit day sub-string, e.g. "#@# Intensity Normalization2 Tue Jan 3 06:30:17 UTC 2023" (instead of "#@# Intensity Normalization2 Tue Jan 17 HH:MM:SS UTC 2023"). This is why we switched from datetime.datetime.strptime to dateutil.parser.parse, which is more robust can handle this case, a while ago (see 073d019 <073d019> ). I suggest that we find a different solution that still considers the local locale settings. Let me know if you have any ideas. — Reply to this email directly, view it on GitHub <#302 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNFF6ASFNF5ZDWCUENFCWDXLAN4FANCNFSM6AAAAAAYFES7KM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[m-reuter]

will merge this for now as it is only a fallback. If any of you has an idea how to do this better, please do another PR.