-
Notifications
You must be signed in to change notification settings - Fork 11
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
Major rework: more pythonic API, code rework #38
Conversation
Codecov Report
@@ Coverage Diff @@
## master #38 /- ##
==========================================
- Coverage 97.67% 97.47% -0.20%
==========================================
Files 1 1
Lines 129 119 -10
==========================================
- Hits 126 116 -10
Misses 3 3
Continue to review full report at Codecov.
|
After thinking about this some more, it probably would make more sense to import PIL.Image gracefully and then check with isinstance/subclass. Otherwise, how would the user load the image in the first place? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
It would be nice if you can split the PIL support, general code cleanup and type hints into different PRs -- this can make code review a lot easier for me :)
I just found that there's no type checking in CI right now, I'll add them tonight.
Totally get WRT splitting up PRs. I'll try to get a different one out for PIL support |
Alright. I removed all the PIL specific code and moved pixelmatch to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please don't checkin pixelmatch/contrib
as they are empty right now.
I had a vague idea that git didn't track empty files/folders, turns out it's only empty folders 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice. Can you rebase or merge master and solves the conflict? Thanks
Done |
https://github.com/Mattwmaster58/pixelmatch-py/runs/657071069?check_suite_focus=true There are some type errors :( |
|
Personally I use editor's auto format: https://github.com/psf/black#editor-integration Python's pre-commit requires manual install, and I have some unpleasant experience with it due to Python venv switching. |
Merged, thanks! |
Thanks for the quick release!
…On Fri, May 8, 2020 at 12:33 PM Wu Haotian ***@***.***> wrote:
https://pypi.org/project/pixelmatch/0.2.0/
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#38 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGI56LPJFOMCTBXGB6446HTRQRF7PANCNFSM4M3RAQCA>
.
--
*Either I will find a way, or I will make one.** - Phillip Sydney*
|
Here's the things I did (which fixes #37 ):
What I didn't do (possibly yet):
What I need to do: