Skip to content

[Refactor] Type fixes and minor improvements to flowlayout.py #880

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.

Sign up for GitHub

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

Jump to bottom
Merged
CyanVoxel merged 5 commits into from
Mar 31, 2025
Merged

[Refactor] Type fixes and minor improvements to flowlayout.py #880

CyanVoxel merged 5 commits into from
Mar 31, 2025

Conversation

@VasigaranAndAngel
Copy link
Collaborator

@VasigaranAndAngel VasigaranAndAngel commented Mar 21, 2025

Summary

  • Added @override decorators and removed # noqa: N802 comments.
  • Changed parameter names in override methods to match the base methods.
  • Used setProperty to assign the "ignore_size" property to FlowWidget.
  • Fixed HeightForWidth to return int instead of float since the base method is annotated to return int.
  • Simplified the if statement in line 127.

Tasks Completed

  • Platforms Tested:
    • Windows x86
    • Windows ARM
    • macOS x86
    • macOS ARM
    • Linux x86
    • Linux ARM
  • Tested For:
    • Basic functionality
    • PyInstaller executable

VasigaranAndAngel
VasigaranAndAngel commented Mar 21, 2025
View reviewed changes
@CyanVoxel CyanVoxel added Type: Refactor Code that needs to be restructured or cleaned up Priority: Low Doesn't require immediate attention Status: Review Needed A review of this is needed labels Mar 21, 2025
@CyanVoxel CyanVoxel moved this to 🏓 Ready for Review in TagStudio Development Mar 21, 2025
Computerdores
Computerdores approved these changes Mar 29, 2025
View reviewed changes
Copy link
Collaborator

@Computerdores Computerdores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all a good improvement, I see no issues besides those two nits.

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Mar 30, 2025
VasigaranAndAngel and others added 3 commits March 31, 2025 20:23
@VasigaranAndAngel @Computerdores
Update src/tagstudio/qt/flowlayout.py
510eabf 
Co-authored-by: Jann Stute <46534683+Computerdores@users.noreply.github.com>
@VasigaranAndAngel @Computerdores
Update src/tagstudio/qt/flowlayout.py
8673dd2 
Co-authored-by: Jann Stute <46534683+Computerdores@users.noreply.github.com>
@VasigaranAndAngel
ruff format
1230f9d
@CyanVoxel CyanVoxel moved this from 🏓 Ready for Review to 🍃 Pending Merge in TagStudio Development Mar 31, 2025
@CyanVoxel CyanVoxel merged commit 7f7d861 into TagStudioDev:main Mar 31, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from 🍃 Pending Merge to ✅ Done in TagStudio Development Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Computerdores Computerdores Computerdores approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Priority: Low Doesn't require immediate attention Type: Refactor Code that needs to be restructured or cleaned up

Projects

TagStudio Development
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@VasigaranAndAngel @Computerdores @CyanVoxel