search

sign-language-processing / pose

Library for viewing, augmenting, and handling .pose files
https://pose-format.readthedocs.io/en/latest/
MIT License
74 stars 23 forks source link

Add Python>3.9 dependency #80

Closed cleong110 closed 6 months ago

cleong110 commented 6 months ago

79 a side discussion here showed that it is currently possible to install pose-format into a python 3.7 environment. However pose-format uses a feature implemented in Python 3.9, the str.removesuffix() function. https://www.geeksforgeeks.org/python-string-removesuffix/

Therefore it would be nice if we could add a dependency check to prevent such installs from happening.

Had a look at https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#dependencies-and-requirements and I guess we'd need a line along the lines of this?

[project]
requires-python = ">= 3.9"
AmitMY commented 6 months ago

Thanks! I see you already committed this, wanna make a pull request?

cleong110 commented 6 months ago

Ah whoops, thought I did that. Try again now

bipinkrish commented 6 months ago

str.removesuffix()

Isn't it better to go with alternative approach for this single thing instead of raising the requirement?

cleong110 commented 6 months ago

@bipinkrish It's a good question! Certainly supporting more versions of Python would be nice, though would take more work.

In my mind, It's not so much that this is a solution to this specific bug, as that we ought to have some minimum Python version, and 3.9 seemed like a decent one to pick given that we know it works on that version with no changes.

As the Python version gets older it becomes harder and harder to maintain support, especially as other required libraries might start imposing their own fixes and requirements. If we require library x but then that library doesn't support old Python versions, etc etc.

Checking the status of different Python versions: https://devguide.python.org/versions/

So I think supporting as far back as 3.9 is pretty generous actually.

Ultimately the question of which versions to support is a separate question, and up to Amit I suppose, as the primary author/maintainer. If he wanted to make it backwards compatible all the way to 3.7 we'd probably have to check for and fix many potential bugs, not just this one issue.

Alternatively we might choose a later version even, such as 3.10 or something in order to minimize the maintenance needed.

All that said if you have a need for 3.7 and think it wouldn't be that difficult to support 3.7 or earlier and want to help, I don't think anyone would turn that down! We'd need to probably add in some more extensive unit testing and such, trying out the library with older versions and making sure everything still installs and runs, etc. Feel free to fork the repo and give it a go!

AmitMY commented 6 months ago

Support is always a tricky question. Adding to @cleong110 - yes, given the current code, we have to specify a minimum version that this code will run on. This does not mean that we are not open to having this run on python 3.7 even - there will have to be a PR that adds 3.7 to the CI tests matrix, then fix all of the bugs an strange behaviors. As long as it does not make the code very bad, I will merge it, otherwise, I'd point people to the branch/fork.

(however, I don't want to use my time in supporting every possible version and programming language etc, as who knows if it might at all be used)