Input validation for Trainer's `min_time` and `max_time` when passed as string

[kylebgorman]

trafficstars

Description & Motivation

The argument to the Trainers min_time is expected to be a string of the form /(\d\d):(\d\d):(\d\d):(\d\d)/. One way in which parsing can fail (simple example: --min_time 60) throws an IndexError and provides little context for what is wrong.

Pitch

This is an uninformative error message, and managed to briefly stump me and a colleague. It should be caught and converted into an informative one that explains the expected format.

This should probably be done at the API level rather than becoming something specific to CLI parsing. That is, since one can provide a string to this API, it should catch this error and instead raise an error with a name like TimeParsingError or something of that nature.

Alternatives

Use datetime.timedelta.strptime or other standard-library elements to do the parsing, and catch the ValueError and raise something more informative.

Additional context

I am working on PyTorch-Lightning >=1.7.0,<2.0.0 because I haven't had the chance to migrate to LightningCLI and such, but a quick perusal suggests this is still an issue.

cc @borda @carmocca @awaelchli

[carmocca]

@kylebgorman Would you like to contribute this suggestion?

[kylebgorman]

@kylebgorman Would you like to contribute this suggestion?

Sure, it seems like a small issue. (I need to clear OSS patches with my employer, but I'll do that now.)