-
-
Notifications
You must be signed in to change notification settings - Fork 394
refactor: move type constants to new media classes #331
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thesacraft
reviewed
Jul 25, 2024
tagstudio/src/core/media_types.py
Outdated
_SPREADSHEET_SET: set[str] = { | ||
".csv", | ||
".numbers", | ||
".ods" ".xls", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a comma missing here
CarterPillow
pushed a commit
to CarterPillow/TagStudio
that referenced
this pull request
Sep 7, 2024
* refactor: move type constants to new media classes * fix: add missing comma + sort extensions
CyanVoxel
added a commit
that referenced
this pull request
Sep 22, 2024
Ports the following thumbnail and related PRs from the `Alpha-v9.4` branch to `main` (v9.5+): - (#273) Blender thumbnail support - (#307) Add font thumbnail preview support - (#331) refactor: move type constants to new media classes - (#390) feat(ui): expanded thumbnail and preview features - (#370) ui: "open in explorer" action follows os name - (#373) feat(ui): preview support for source engine files - (#274) Refactor video_player.py (Fix #270) - (#430) feat(ui): show file creation/modified dates + restyle path label - (#471) fix(ui): use default audio icon if ffmpeg is absent - (#472) fix(ui): use birthtime for creation time on mac & win Co-Authored-By: Ethnogeny <111099761+050011-code@users.noreply.github.com> Co-Authored-By: Theasacraft <91694323+Thesacraft@users.noreply.github.com> Co-Authored-By: SupKittyMeow <77246128+supkittymeow@users.noreply.github.com> Co-Authored-By: EJ Stinson <93455158+favroitegamers@users.noreply.github.com> Co-Authored-By: Sean Krueger <71362472+seakrueger@users.noreply.github.com>
CyanVoxel
added a commit
that referenced
this pull request
Oct 7, 2024
* feat: port v9.4 thumbnail + related feats to v9.5 Ports the following thumbnail and related PRs from the `Alpha-v9.4` branch to `main` (v9.5+): - (#273) Blender thumbnail support - (#307) Add font thumbnail preview support - (#331) refactor: move type constants to new media classes - (#390) feat(ui): expanded thumbnail and preview features - (#370) ui: "open in explorer" action follows os name - (#373) feat(ui): preview support for source engine files - (#274) Refactor video_player.py (Fix #270) - (#430) feat(ui): show file creation/modified dates + restyle path label - (#471) fix(ui): use default audio icon if ffmpeg is absent - (#472) fix(ui): use birthtime for creation time on mac & win Co-Authored-By: Ethnogeny <111099761+050011-code@users.noreply.github.com> Co-Authored-By: Theasacraft <91694323+Thesacraft@users.noreply.github.com> Co-Authored-By: SupKittyMeow <77246128+supkittymeow@users.noreply.github.com> Co-Authored-By: EJ Stinson <93455158+favroitegamers@users.noreply.github.com> Co-Authored-By: Sean Krueger <71362472+seakrueger@users.noreply.github.com> * remove vscode exceptions from `.gitignore` * delete .vscode directory * style: format for `ruff check` * fix(tests): update `test_update_widgets_not_selected` test * remove Send2Trash dependency * refactor: use dataclass for MediaCateogry * refactor: use enums for UI colors * docs: add file docstring for silent_Popen * refactor: replace logger with structlog * use early return inside `ResourceManager.get()` * add `is_ext_in_category()` method to `MediaCategory` Add method to check if an extension is a member of a given MediaCategory. * style: fix docstring style, missing type hints, rename `afm` * fix: use structlog vars in logging * refactor: move platform-dependent strings to PlatformStrings * refactor: move `parents[2]` path to variable * fix: undo logger regressions --------- Co-authored-by: Ethnogeny <111099761+050011-code@users.noreply.github.com> Co-authored-by: Theasacraft <91694323+Thesacraft@users.noreply.github.com> Co-authored-by: SupKittyMeow <77246128+supkittymeow@users.noreply.github.com> Co-authored-by: EJ Stinson <93455158+favroitegamers@users.noreply.github.com> Co-authored-by: Sean Krueger <71362472+seakrueger@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Priority: Medium
An issue that shouldn't be be saved for last
Status: Review Needed
A review of this is needed
Type: Refactor
Code that needs to be restructured or cleaned up
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactor is meant to provide an improved interface for determining media categories from file extensions. Instead of relying on searching though lists of extension constants, this refactor provides a new
MediaCategory
class, which in turn is composed of aMediaType
enum, a set of extensions defined by one or more internal sets, and additional data such as IANA registration (more on that later).The
MediaCategory
class is then used by theMediaCategories
class, which contains every pre-built category used by the program along with theget_types
method, which is the new intended way to check for media category membership. Instead of checking if extension.foo
is inside aBAR_TYPES
list constant,foo
is passed to theget_types
method which returns anyMediaType
matches from the extension sets inside ofMediaCategory
objects added inside theALL_CATEGORIES
list.While I originally intended to rely heavily on MIME types rather than curated extension lists, it became apparent to me that this wouldn't be feasible for the use cases of TagStudio. There are cases where multiple media categories are desired, such as
.csv
in bothPLAINTEXT
andSPREADSHEET
categories. In addition, MIME type categories on their own are too limited in scope for some of categorization I'd like to achieve, such as with Office files. With that being said I still wanted the ability to leverage MIME types where possible. Theget_types
method has an optional flag that when true will leverage themimetypes
module and attempt to guess the media category of an extension if a match was not found in one of the built-in sets. The IANA registration flag within theMediaCategory
class is then used to prevent theget_types
method from doing unnecessary membership checks inside otherwise unregistered media categories.TL;DR: New
MediaType
enums andget_types
method to replace the extension list constants with a more flexible system.