globocom / m3u8

Python m3u8 Parser for HTTP Live Streaming (HLS) Transmissions
Other
1.98k stars 464 forks source link

Use built-in ISO 8601 parsing #331

Closed daveisfera closed 8 months ago

daveisfera commented 8 months ago

Continuing in the vein of #38, the built-in parsing of ISO 8601 that was added in Python 3.7 should be used. This would require bumping the minimum supported Python version, but 3.8 is the oldest version that's still supported, so that should be a fine thing to do. I'd be glad to submit a PR for this, if it will be accepted

bbayles commented 8 months ago

One thing to consider: if you do performance profiling, you'll find that date parsing is one of the slowest things this library does. The ciso8601 package is a C extension that speeds up this process greatly.

Right now it's easy to patch in ciso8601 for iso8601 (m3u8.parser.cast_date_time = ciso8601.parse_datetime).

I support dropping the iso8601 dependency, but it would be good to maintain the ability to patch in ciso8601.

daveisfera commented 8 months ago

According to the benchmarks on ciso8601, it's a significant improvement over iso8601 (almost 100x), but compared to the built-in function it's a lot closer, so the need to change that should go down a lot with this change and would still be completely possible

daveisfera commented 8 months ago

On a semi-related note, I added a PR for adding 3.12 to the list of tested versions and I noticed that ciso8601 doesn't have wheel packages for that yet, so it would need to be built from source until those are added

daveisfera commented 8 months ago

Looks like this isn't a viable solution until Python 3.11 so I'm guessing this needs to wait

movermeyer commented 8 months ago

FWIW, there is also the option of using backports.datetime_fromisoformat, which brings Python 3.11's parsing code to older versions of Python 3 (with pre-built wheels and high-performance).

daveisfera commented 8 months ago

I did another version of my PR that uses the built-in on 3.11 and later, but I really liked the idea of using the backports (for the consistent implementation and performance improvement), so I added that to the PR as well. Thanks for the suggestion!