Skip to content

Conversation

Computerdores
Copy link
Collaborator

Summary

Mostly type fixes in renderer also some other small ones. Split from #1113 to keep PR size on the smaller end.

Tasks Completed

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

@CyanVoxel CyanVoxel added this to the Alpha v9.5.6 milestone Sep 8, 2025
@CyanVoxel CyanVoxel added the Type: Refactor Code that needs to be restructured or cleaned up label Sep 8, 2025
@Computerdores Computerdores moved this to 🏓 Ready for Review in TagStudio Development Sep 9, 2025
@Computerdores Computerdores added the Status: Review Needed A review of this is needed label Sep 9, 2025
@Computerdores
Copy link
Collaborator Author

fixed merge conflict

@CyanVoxel CyanVoxel moved this from 🏓 Ready for Review to 👀 In review in TagStudio Development Sep 9, 2025
@Computerdores
Copy link
Collaborator Author

for me all pyright issues are fixed now. Only thing I am not a particular fan of is the number of ignore comments on get calls to the ResourceManager, but didn't know a better solution off the top of my head

@CyanVoxel
Copy link
Member

Only thing I am not a particular fan of is the number of ignore comments on get calls to the ResourceManager, but didn't know a better solution off the top of my head

My longer term solution is to overhaul that thing to have different getters deepening on the type you expect. I have no idea what's going on with the type stuff over there, only that it's guaranteed that no other part of the code can ever be happy with the type it spits out...

elif ext in [".mp4", ".m4a", ".aac"]:
mp4_tags: mp4.MP4 = mp4.MP4(filepath)
mp4_covers: list = mp4_tags.get("covr")
mp4_covers: list = mp4_tags.get("covr") # pyright: ignore[reportAssignmentType]
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, are you also seeing the reportUnknownVariableType warning for these lines?
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't otherwise I would have set it to ignored. Only have nvim available today though, so can't check

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Sep 10, 2025
@CyanVoxel CyanVoxel moved this from 👀 In review to 🍃 Pending Merge in TagStudio Development Sep 10, 2025
@CyanVoxel CyanVoxel added the Status: Mergeable The code is ready to be merged label Sep 10, 2025
@CyanVoxel CyanVoxel removed the Status: Mergeable The code is ready to be merged label Sep 11, 2025
@CyanVoxel CyanVoxel merged commit c6f6697 into TagStudioDev:main Sep 11, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from 🍃 Pending Merge to ✅ Done in TagStudio Development Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Refactor Code that needs to be restructured or cleaned up

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants