Conversation
| height=t_height, | ||
| method=t_method, | ||
| type=t_type, | ||
| length=t_byte_source.tell(), |
There was a problem hiding this comment.
Sanity check: is t_byte_source definitely going to be fully written with the cursor at the end of the buffer, so that .tell() is the length? Both .crop() and .scale() say that they return "bytes ... ready to be written to disk" so I assume this is fine.
There was a problem hiding this comment.
My understand is that it will be written to and the cursor will be at the end of buffer; we could do something like:
cur = t_byte_source.tell()
t_byte_source.seek(0, os.SEEK_END)
end = t_byte_source.tell()
t_byte_source.seek(cur, os.SEEK_SET)If you're concerned about it not being at the end of the buffer.
There was a problem hiding this comment.
Reading back through the code I think this is fine TBH.
There was a problem hiding this comment.
That was my understanding too, I just wanted to check this through. Tinkering in an interpreter, our understanding seems to be correct:
20230728 (Red Hat 13.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from tests.test_utils import SMALL_PNG
>>> from io import BytesIO
>>> src = Image.open(BytesIO(SMALL_PNG))
>>> output = BytesIO()
>>> src.save(output, "png")
>>> output.tell()
70
>>> len(output.getvalue())
70Though curiously this is slightly larger than the original PNG:
>>> len(SMALL_PNG)
67
| type_quality = desired_type != info["thumbnail_type"] | ||
| length_quality = info["thumbnail_length"] | ||
| type_quality = desired_type != info.type | ||
| length_quality = info.length |
There was a problem hiding this comment.
Note: this is duplicated out of the info struct because everything but the last field is used as a sort criterion.
There was a problem hiding this comment.
Right -- I guess we could just inline it below, but I'm trying to avoid too much refactoring at the same time as this?
| "thumbnail_method": method, | ||
| "thumbnail_type": self.test_image.content_type, | ||
| "thumbnail_length": 256, | ||
| "filesystem_id": f"thumbnail1{self.test_image.extension.decode()}", |
There was a problem hiding this comment.
Was filesystem_id just redundant here? (Guessing it should have been in FileInfo and was missed in some previous refactor?)
There was a problem hiding this comment.
get_remote_media_thumbnails no longer returns it, we're just matching the return type of that function here.
Use attrs classes in more spots instead of dictionaries.
Kind of related to #16431.