Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Working on modernizing - some queries #93

Open
demberto opened this issue Jul 5, 2023 · 3 comments
Open

Working on modernizing - some queries #93

demberto opened this issue Jul 5, 2023 · 3 comments

Comments

@demberto
Copy link

demberto commented Jul 5, 2023

Do you want to:

  • ... drop Python 2 support (and make 3.7 the new minimum required version)?
  • ... migrate to pyproject.toml? (see PEP621)
  • ... add inplace type hints / annotations? (see PEP561)
  • ... migrate to click for the srt_tools? (all scripts can be grouped into subcommands like how git does)
  • ... use attrs for Subtitle class instead of hand-coding all dunder methods?
@cdown
Copy link
Owner

cdown commented Jul 6, 2023

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
Copy link
Author

demberto commented Jul 6, 2023

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
Copy link
Contributor

smacke commented Jul 6, 2023

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 :) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants