Skip to content

Cleanup dithering conditional and explanation#41

Merged
kageru merged 6 commits into
Irrational-Encoding-Wizardry:masterfrom
OrangeChannel:dither_cond
Jun 23, 2020
Merged

Cleanup dithering conditional and explanation#41
kageru merged 6 commits into
Irrational-Encoding-Wizardry:masterfrom
OrangeChannel:dither_cond

Conversation

@OrangeChannel
Copy link
Copy Markdown
Contributor

Shouldn't change anything because calling resize with float output and a dither_type disregarded the dither_type value anyways. Reverts some of the changed behavior in #39 and avoids the misleading should_dither being true for float sample type.

Shouldn't change anything because calling resize with float output and
a dither_type disregarded the dither_type value anyways.
Comment thread vsutil/__init__.py Outdated
@stuxcrystal
Copy link
Copy Markdown
Contributor

We are at our third PR with discussion about this topic iirc.

At this point I think we should split the one-liner into multiple lines and put them into a (possibly internal) function.

@OrangeChannel OrangeChannel requested a review from FichteFoll June 21, 2020 22:21
@OrangeChannel
Copy link
Copy Markdown
Contributor Author

This can get squashed into something like fix dithering logic (#41) and tagged as 0.3.1. I'll then rebase #37 ontop of this since it shares some of the test code and depth needs the decorator too.

Or merge #37 first (tagged as 0.4.0) and then I'll rebase this and get this tagged as 0.4.1 (which should be the next pypi/package release).

Comment thread vsutil/__init__.py Outdated
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
Copy link
Copy Markdown
Member

@kageru kageru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are fine, but _should_dither is way too enterprise. Just make it a wrapper around the boolean condition that we had before, optionally with some nicer names like you already did here, but don’t give it all those pseudo-optional parameters where only some combinations of them are actually valid.

Comment thread vsutil/__init__.py Outdated
Comment thread vsutil/__init__.py Outdated
OrangeChannel and others added 3 commits June 23, 2020 14:03
The order makes a bit more sense now and the ability to use a clip
is gone.
@kageru kageru merged commit 6a33c93 into Irrational-Encoding-Wizardry:master Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants