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

Explicitly type Image __init__ attributes #8275

Closed
wants to merge 3 commits into from

Conversation

Yay295
Copy link
Contributor

@Yay295 Yay295 commented Aug 1, 2024

No description provided.

@Yay295 Yay295 changed the title Explicitly type Image._size Explicitly type Image __init__ properties Aug 1, 2024
@Yay295 Yay295 changed the title Explicitly type Image __init__ properties Explicitly type Image __init__ attributes Aug 1, 2024
@Yay295
Copy link
Contributor Author

Yay295 commented Aug 1, 2024

Unfortunately, trying to type im and palette leads to a lot of false errors because mypy can't always determine that those values aren't None.

Related:
python/mypy#10151
python/mypy#11969

@radarhere
Copy link
Member

I've created #8279 as an alternative to this, addressing im and palette as well.

@Yay295
Copy link
Contributor Author

Yay295 commented Aug 2, 2024

Other than the im and palette changes, the only differences I see are a slightly different handling of PngImagePlugin, and a very different handling of EpsImagePlugin.

@radarhere
Copy link
Member

I think my versions of the EPS and PNG changes are simpler. I've also restricted the hinting of self.info keys more than this PR.

@Yay295
Copy link
Contributor Author

Yay295 commented Aug 3, 2024

I'm not sure how you think your PNG change are simpler when you introduce a new local variable and I don't.

@radarhere
Copy link
Member

My opinion is that it's simpler because my version declares what the type is once, whereas this does it three times.

@radarhere
Copy link
Member

#8279 has been merged instead.

@radarhere radarhere closed this Aug 28, 2024
@Yay295 Yay295 deleted the image_init_typing branch August 28, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants