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.

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

Fix comparison of "numpy" and "pil" types in Image #364

Merged
merged 2 commits into from Nov 15, 2021
Merged

Fix comparison of "numpy" and "pil" types in Image #364

merged 2 commits into from Nov 15, 2021

Conversation

BioGeek
Copy link
Contributor

@BioGeek BioGeek commented Nov 15, 2021

The line

elif self.type == "numpy" or "pil":

is interpreted as

elif (self.type == "numpy") or ("pil"):

where ("pil") is always evaluated as True. See https://stackoverflow.com/a/20002504/50065 for more info.

The reason this wasn't caught by the tests is because they were written like:

  with self.assertRaises(ValueError):
      wrong_type = gr.inputs.Image(type="unknown")
      wrong_type.preprocess(img)
      wrong_type.serialize("test/test_files/bus.png", False)

The line wrong_type.preprocess(img) would trigger a ValueError and the test would be markes as succeeded, but it obscures that the line wrong_type.serialize("test/test_files/bus.png", False) would fail with:

Traceback (most recent call last):
  File "/home/biogeek/github/gradio/test/test_inputs.py", line 320, in test_as_component
    wrong_type.serialize("test/test_files/bus.png", False)
  File "/home/biogeek/github/gradio/gradio/inputs.py", line 768, in serialize
    "."+fmt.lower() if fmt is not None else ".png"))
AttributeError: 'builtin_function_or_method' object has no attribute 'lower'

@height
Copy link

height bot commented Nov 15, 2021

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@abidlabs
Copy link
Member

Thank you so much @BioGeek for spotting this and explaining the fix. LGTM

@abidlabs abidlabs merged commit 18617f3 into gradio-app:master Nov 15, 2021
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.

None yet

2 participants