Clean up tests #6653
Clean up tests #6653
Conversation
|
Thanks for this PR! The first five commits look good. "use tuple or set instead of list in tests" is too big to review properly, and I know tuples are generally preferred to lists, but in this case I don't think the huge review effort is worth any noticeable benefit. (I think these kind of changes are okay when making other changes to a file.) And @radarhere had a question about "sort colors before comparing them", so I'll defer on that. Also I wouldn't worry about the last few commits splitting long lines and wrangling with the formatting, it's okay to have longish strings (below the limit) as they're easier to grep. |
| @@ -57,13 +57,13 @@ def test_convolution_modes(self): | |||
| Image.Resampling.LANCZOS, | |||
| ), | |||
| ) | |||
| def test_reduce_filters(self, resample): | |||
| r = self.resize(hopper("RGB"), (15, 12), resample) | |||
There was a problem hiding this comment.
Why is 'resampling_filter' clearer? 'resample' is the name of the variable when the core resize operation is called in Image.py
Line 2192 in 86634b8
There was a problem hiding this comment.
Because "resample" is an action, while "resampling filter" is an object. The thing being passed is (an ID for) a resampling filter, not a "resample". It doesn't work to say that "resample" is the type of the thing being passed either, because the type of the thing being passed is a Resampling enum item.
|
Those PRs have all now been merged. |
Assert statements give a better message this way.
|
I've rebased it. Should I create a new pull request for this, or keep using this one? |
|
This PR is fine. I don't think we really need 3f045ea "use tuple or set instead of list in tests", there's a lot of changes (+406 −399 in 54 files) for little benefit. |
|
I've moved the |
No description provided.