Support SizeDict import in get_size_dict#44903
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| size (`int | Iterable[int] | dict[str, int] | SizeDict`, *optional*): | ||
| The `size` parameter to be cast into a size dictionary. | ||
| max_size (`int | None`, *optional*): | ||
| The `max_size` parameter to be cast into a size dictionary. |
There was a problem hiding this comment.
Does this mean that size and max_size are mutually exclusive? If yes you should add an error if they are both non-None
| if isinstance(size, dict): | ||
| return size |
There was a problem hiding this comment.
This block will never be entered because of
if not isinstance(size, dict):
size_dict = convert_to_size_dict(size, max_size, default_to_square, height_width_order)There was a problem hiding this comment.
Yes maybe I was overly cautious there, since users used get_size_dict directly which I didn't expect, I was thinking that maybe some also import and use convert_to_size_dict directly. But dict and SizeDict inputs weren't supported originally in convert_to_size_dict so that's probably not necessary.
Will revert this and move the SizeDict check to get_size_dict
| if isinstance(size, SizeDict): | ||
| return dict(size) |
There was a problem hiding this comment.
Should this be a condition in get_size_dict? It might be confusing if we end up entering this function with non-default values of max_size, default_to_square, height_width_order and they have no effect.
There was a problem hiding this comment.
Yes indeed, I'll revert this as mentioned above
9dd1332 to
6e25c13
Compare
What does this PR do?
Some remote code models are using
get_size_dictdirectly, and now that size is converted to SizeDict in init, we need to support it as input inget_size_dict