Skip to content

Refactor and Fixes: Type Hints, Import Issues, and Code Formatting in UI code base #605

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
Closed
wants to merge 4 commits into from
Closed

Refactor and Fixes: Type Hints, Import Issues, and Code Formatting in UI code base #605

wants to merge 4 commits into from

Conversation

VasigaranAndAngel
Copy link
Collaborator

  • Refactored code to address type hint issues across multiple files.
  • Fixed import issues.
  • Resolved mypy and pytest errors.
  • Formatted with ruff.
  • Removal of few unnecessary lambdas.
  • Removal of home_ui.py, home.ui files.

@CyanVoxel CyanVoxel added Type: Refactor Code that needs to be restructured or cleaned up Priority: Low Doesn't require immediate attention Priority: Medium An issue that shouldn't be be saved for last Status: Review Needed A review of this is needed labels Nov 28, 2024
Computerdores
Computerdores reviewed Nov 30, 2024
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.

Just a couple comments, all in all a lot of very nice refactoring though!

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Nov 30, 2024
@VasigaranAndAngel
Refactor and Fixes: Type Hints, Import Issues, and Code Formatting
33b80b4 
- Refactored code to address type hint issues across multiple files.
- Fixed import issues.
- Resolved `mypy` and `pytest` errors.
- Formatted with ruff.
- Removal of few unnecessary lambdas.
- Removal of `home_ui.py`, `home.ui`

refactor: in main_window.py, removed 'Qt User Interface Compiler' warning and unwanted functions.
@VasigaranAndAngel
addressing request: changed filepath.as_posix() to str(filepath)
4eca516
@VasigaranAndAngel
fix: type hint fixes
e9d8912
@CyanVoxel CyanVoxel removed the Priority: Medium An issue that shouldn't be be saved for last label Mar 5, 2025
@CyanVoxel
Copy link
Member

Hey! I really appreciate your work on this, it cleans up and fixes a lot of nice stuff in the project. However due to the nature of far-reaching refactors like this, it pretty quickly started to accrue merge conflicts as development was focused on wrapping up 9.5. This always ends up happening to PRs of this nature, which is why I have a notice in the contributing.md about refactors like this.

Nonetheless, there's some good gems in here that I've taken note of for the future. Many of the changes in here have also already made their way into the program as part of little cleanup efforts that naturally fit into relevant PRs for the affected areas. Some of these changes implemented include:

  • Various updated type hints, including nullables
  • Removed type hints from enums
  • Fix landing.py set_status_label() type hint bug
  • Preview Panel changes
  • tag_box.py changes

As for great changes here that still haven't been added elsewhere, there's:

  • Remove home_ui.py and home.ui
  • Add remaining nullable hints
  • Fix FileOpenerLabel init() type hint bug
  • Fix linear_gradient() type hint bug
  • General main_window.py cleanup
  • Folders to Tags fixes (Now deprecated feature)
  • Removing lambdas from menubar calls (though historically there's been some issues with this, needs testing)

There's also a few other changes in here that I don't think should be added in the future, including the added assert statements, cached get() results in the resource manager, and the inline vs list import changes, all for various reasons.

Overall though, this PR has been a great help at informing improvements to the project, and continues to be so - even if unfortunately I'm afraid it's past the point of pulling due to the numerous conflicts. Some smaller, more focused PRs focusing on the remaining changes would be the way to go moving forward, if the changes themselves don't already get implemented in dedicated feature PRs.

Thank you again for your work on this, I really appreciate your contributions here!

@CyanVoxel CyanVoxel closed this Mar 12, 2025
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in TagStudio Development Mar 12, 2025
CyanVoxel added a commit that referenced this pull request Mar 12, 2025
@CyanVoxel @VasigaranAndAngel
refactor: remove unused ui files
234ddec 
Remove unused UI files (Port #605)

Co-Authored-By: VasigaranAndAngel <72515046+VasigaranAndAngel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Computerdores Computerdores Computerdores left review comments

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 @CyanVoxel @Computerdores