google / gif-for-cli

https://opensource.googleblog.com/2018/06/tenor-gif-for-cli.html
Apache License 2.0
2.93k stars 161 forks source link

Use `statistics.mean` instead of custom `avg` function? #13

Closed g-mc closed 6 years ago

g-mc commented 6 years ago

https://github.com/google/gif-for-cli/blob/288b7b3ae88661ca805ac8c6b574a8448bcf37d9/gif_for_cli/generate/utils.py#L29-L31

Looking at the avg function, I was curious about potentially using statistics.mean instead. The functionality between the two seemed the same except for avg always returning a float VS statistics.mean sometimes returning an integer (where applicable).

>>> avg([1])
1.0
>>> mean([1])
1
>>> avg([1, 2, 5])
2.6666666666666665
>>> mean([1, 2, 5])
2.6666666666666665

The main issue was that statistics.mean requires Python 3.4+ but the installation instructions in the README only mentioned Python 3:

Requires Python 3 (with setuptools and pip) ...

Looking at the docs for the dependency Pillow seemed to indicate that 3.4 would be the minimum supported version for gif-for-cli:

Pillow >= 5.0.0 supports Python versions 2.7, 3.4, 3.5, 3.6

As long it's acceptable to have an integer returned in some cases instead of always returning a float, removing the avg function allows some code and tests to be removed.


Other changes:

SeanHayes commented 6 years ago

Closed & reopened to run new CI check.

There's now a conflict that needs to e fixed, sorry.