Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Keep ICC profile when saving image, if present #554

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Keep ICC profile when saving image, if present #554

wants to merge 2 commits into from

Conversation

siovene
Copy link
Contributor

@siovene siovene commented Oct 2, 2020

Closes #366

cogat
cogat suggested changes Oct 3, 2020
View reviewed changes
Copy link

@cogat cogat left a comment

Choose a reason for hiding this comment

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

(I don't have a commit bit so may be unqualified).

This is a useful feature, quite neatly implemented. I would like to see if it can be done without altering the signature of save_image and hence needing to alter the tests.

easy_thumbnails/engine.py Outdated Show resolved Hide resolved
easy_thumbnails/engine.py Outdated Show resolved Hide resolved
easy_thumbnails/engine.py Outdated Show resolved Hide resolved
easy_thumbnails/engine.py Outdated Show resolved Hide resolved
easy_thumbnails/tests/test_engine.py Outdated Show resolved Hide resolved
easy_thumbnails/engine.py Show resolved Hide resolved
@siovene
Copy link
Contributor Author

siovene commented Oct 7, 2020

Hi @cogat, thanks for the review! I think I addressed all points. They were very valid and it looks a lot better now :) Can you give it another look, please?

Sorry for the number of commits, I assume there will be a squash & merge at the end.

Thanks!

@siovene
Copy link
Contributor Author

siovene commented Jun 23, 2021

Just for piece of mind, this PR has been in production on AstroBin for half a year with no problems :)

@siovene
Copy link
Contributor Author

siovene commented Jul 6, 2021

@SmileyChris any chance this could be reviewed? It's the only thing that keeps me from using the upstream library, and it seems to work fine: it's been in production on AstroBin for half a year, which is a volume of about 50k images. Thanks!

@ataylor32 ataylor32 mentioned this pull request Dec 7, 2022
Open
@BigglesZX
Copy link

Any chance of getting this merged in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cogat cogat cogat requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Images lose their icc_profile
3 participants
@siovene @BigglesZX @cogat