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

Simplified code #5315

Merged
merged 3 commits into from Jun 30, 2021
Merged

Simplified code #5315

merged 3 commits into from Jun 30, 2021

Conversation

@radarhere
Copy link
Member

@radarhere radarhere commented Mar 6, 2021

I think this change simplifies the code. Feel free to disagree.

src/PIL/Image.py Outdated
return (
self.convert("La")
self.convert(self.mode.replace("A", "a"))

This comment has been hidden.

@wiredfool
Copy link
Member

@wiredfool wiredfool commented Mar 6, 2021

I don't like this, while it does remove a duplicated bit of code, it obscures what's happening.

In my opinion,

  • Modes are essentially enums that happen to be strings. Doing string.replace is kind of weird on an enum.
  • It reduces greppability. It's nice to be able to grep the code and find everywhere La is referenced.

If really want to do something to remove the duplicate code, perhaps a dict with a mapping of {'LA': 'La', 'RGBA':'RGBa'} would be appropriate.

@radarhere
Copy link
Member Author

@radarhere radarhere commented Mar 7, 2021

I've pushed a commit to use the mapping option, and in two other locations as well.

hugovk
hugovk approved these changes Jun 28, 2021
@radarhere radarhere merged commit cab9179 into python-pillow:master Jun 30, 2021
50 checks passed
@radarhere radarhere deleted the simplified branch Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants