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

Refactor Thumbrenderer #168

Merged
merged 7 commits into from
May 18, 2024

Conversation

Thesacraft
Copy link
Collaborator

@Thesacraft Thesacraft commented May 13, 2024

I refactored Thumbrenderer to remove duplicate code.
I merged the render and render_big methods into one _render method
I'm open to any suggestions and changes

@CyanVoxel CyanVoxel modified the milestones: Alpha 9.3.x, Alpha 9.2.1 May 15, 2024
@CyanVoxel
Copy link
Member

Overall this looks pretty good to me! The thumb renderer is about to be sort of a hot spot in the code with lots of activity (#171, #175, #179, #183, raw-image-previews, etc.), so while I'm open to an even larger refactor things will probably need to be done in smaller chunks over time after this PR. Consolidating the render and render_big methods was the huge thing that needed done though, so I'm happy to have that improved in time for the next round of fixes and additions!

With all that being said, I'll be pulling #156 before this which refactors out all uses of os.path in favor of the pathlib library going forward. I'll also be pulling the MyPy workflow from #169 which will also shake things up here. I apologize for the additional refactoring/formatting.

CyanVoxel and others added 3 commits May 18, 2024 16:27
- Change all instances of "os.path.normpath" to pathlib's "Path"
- Remove unused import
- Modify log formatting
- Change "self.tr" to "self.thumb_renderer" to avoid masking internal method
- Restore DecompressionBombError handling from main
- Misc. formatting
@CyanVoxel CyanVoxel merged commit e94c487 into TagStudioDev:main May 18, 2024
3 checks passed
@Thesacraft
Copy link
Collaborator Author

Thanks for resolving the conflicts. i was just waiting for the os.paths pr to get merged and do it all in one big cleanup, haha. But i really appreciate the help.

@Thesacraft Thesacraft deleted the Refactor-Thumbrenderer branch May 18, 2024 23:59
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