search

cdown / srt

A simple library and set of tools for parsing, modifying, and composing SRT files.
MIT License
464 stars 44 forks source link

Working on modernizing - some queries #93

Open demberto opened 1 year ago

demberto commented 1 year ago

Do you want to:

cdown commented 1 year ago

Thanks for looking at this! I'd welcome some modernisation and appreciate you bringing this up.

... drop Python 2 support (and make 3.7 the new minimum required version)?

I was going to say we still have some downstream users we care about still support Python 2, but now I see that ffsubsync that I was thinking of in particular supports from 3.6+ only. That said, it also doesn't buy us much to move -- there aren't really any Python 3 language constructs that would help a lot here. Cc @smacke

... migrate to pyproject.toml? (see PEP621)

I'm writing a lot less Python than I used to, so while I've seen these around, I'm a bit lacking on context on how it would benefit srt. Could you maybe explain a bit what kind of things it might help with? :-)

... add inplace type hints / annotations? (see PEP561)

I'd like type hinting, the only question is Python 2 backwards compatibility (assuming we still keep it).

... migrate to click for the srt_tools? (all scripts can be grouped into subcommands like how git does)

Probably not, since these also serve as simple examples and I suspect grouping them would somewhat complicate legibility there. Although maybe I'm overestimating the decrease in legibility?

... use attrs for Subtitle class instead of hand-coding all dunder methods?

We currently have no external dependencies for srt. If we already had some I'd not care as much, but going from 0 to 1 is a larger step. Do you think this would help substantially?

Thanks for writing this up!

demberto commented 1 year ago

Type hints

Ok till you decide whether Python 2 compatibility should be retained, I can make a separate stub file for annotations. This way type checkers get satisfied and also the codebase remains the same.

PEP621

It brings forward a new way to declare project metadata. For simple projects like srt that means just moving from setup.py (which is deprecated) to pyproject.toml. Whatever build system you use (flit, hatch, setuptools), the PEP621 standardises the project metadata. See Declaring project metadata for more info.

srt_tools

It should probably as-is right now, but I highly recommend you to look at click, or better typer.

smacke commented 1 year ago

I was going to say we still have some downstream users we care about still support Python 2, but now I see that ffsubsync that I was thinking of in particular supports from 3.6+ only. That said, it also doesn't buy us much to move -- there aren't really any Python 3 language constructs that would help a lot here. Cc @smacke

Supporting Python 3.6 is a pain. I'll gladly drop support if there's a good excuse (such as srt no longer supporting it :) )