algoo / preview-generator

generates previews of files with cache management
https://pypi.org/project/preview-generator/
MIT License
235 stars 51 forks source link

Fix: 293 #294

Closed a523 closed 2 years ago

a523 commented 2 years ago

https://github.com/algoo/preview-generator/issues/293

PR Goal

fix bug https://github.com/algoo/preview-generator/issues/293

Implemented solution

Checkpoints

a523 commented 2 years ago

@inkhey Please take a look

inkhey commented 2 years ago

@a523 Thanks for this contribution. Just for information, before the introduction of the auto-rotate you introduced during switching to pillow, we consider an other way to do so by copying exif information : https://github.com/algoo/preview-generator/issues/166. This seems a bit more complicated, so i will check and validate your PR to have a consistent behavior for now, but we may decide to change this a bit later. I personally don't have strong opinion which solution is better.

inkhey commented 2 years ago

Properly tested, and fixed mypy issue on branch ci_test/fix293. Can you merge it in your branch, so i can merge this PR ?

to avoid such mypy in CI, you can do : mypy --ignore-missing-imports --disallow-untyped-defs . before suggesting PR.

The problem seems that pre-commit config (https://github.com/algoo/preview-generator/blob/develop/.pre-commit-config.yaml) does not contain mypy whereas CI check it (https://github.com/algoo/preview-generator/blob/develop/concourse/scripts/lint).

a523 commented 2 years ago
  1. certainly, I have merged.
  2. I'm sorry, I did forgot to run mypy checks, will pay attention next time. @inkhey
inkhey commented 2 years ago

@a523 You're welcome. Thanks ;-)