Conversation
|
Hey @cabreraalex , thanks for creating a PR. One minor comment, can you please update your PR with the changes that are not relevant to this PR? Seems like there are some spacing changes, single/double quotes changes, etc. |
|
oops yup, done |
There was a problem hiding this comment.
@cabreraalex Thanks for adding this! Would be great to have a unit test for this functionality as well before merging.
|
@cabreraalex Mind addressing the review comments when you have some time? Thanks! |
|
AFAI can tell just the config is passed, e.g. from jpeg:12 only the 12 is passed to from_str: There is no formal type for the quality, just integer |
| self.quality = quality | ||
|
|
||
| @classmethod | ||
| def from_str(cls, config: str) -> Self: |
There was a problem hiding this comment.
We should probably also add a test for this too then, to also confirm what it should look like when used. Thanks!
|
@cabreraalex Mind adding the test that @ethantang-db mentioned and we can get this in? |
Description of changes:
Adds option for JPEG quality.
Issue #, if available:
Merge Checklist:
Put an
xwithout space in the boxes that apply. If you are unsure about any checklist, please don't hesitate to ask. We are here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
pre-commiton my change. (check out thepre-commitsection of prerequisites)