-
Notifications
You must be signed in to change notification settings - Fork 387
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
feat!: tag categories #655
Conversation
While you are already changing the DB, you might want to also fix the |
I would love to do this 👍 |
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.
Had some free time and wanted to get ahead of the curve a little bit as this PR seems to be shaping up to be a big one.
Feel free to ignore for now, the PR is still a draft after all.
Couldn't resist so I tested it a little, when I have a tag Then when I create a tag
|
I believe Cyan is currently working on fixing that |
Awesome! 👍 |
You just happened to catch me as I (hopefully) finally just got this working in the latest commit! 😁 |
I checked out the last commit and it works as expected! Problematic CaseLets say I have a library of books, and these books have characteristics, the themes are a category, the setting is a category, the time period as well As a result, the Might be a bad example, but it's related to bundling of tags under one tag, and the tags that are bundled are part of several categories I also don't have an immediate answer as to how those should be displayed instead, but I just wanted to point out a problem with displaying sub tags under the categories of their parent tags IdeasOne easy-ish(?) solution would be to not display tags under a category if the tag itself is a category Another idea would be to add another checkbox that disables categorization for a tag, so they only show under the A third idea could be to disassemble the tag, the category would only contain the direct child, letting the further descendants be displayed under |
I think this is more so an issue that arises with that approach to tagging rather than the category system. Parent Tags are intended to be used in an inheritance type of relationship with other tags. This works well for books that are certain genres, but not for describing someone like "John Doe" as "Science Fiction" because he writes in that genre often. "John Doe" may have some association with the genre, but he isn't that genre in the same way that a book may be a genre, or how another genre such as "Hard Sci-Fi" may also be and inherit from "Science Fiction". Having genre parent tags on authors themselves would also break as soon as you came across a book by them that no longer fell under that genre - there would no longer be a way to get rid of that genre from the file entry. Note: For 9.6 I'm planning to introduce a new type of subtag in addition to Parent Tags called "Component Tags" which will use a composition/HAS relationship instead of the inheritance/IS relationship that Parent Tags use.
In this case wouldn't you be marking "John Doe" and every other author as categories? That wouldn't be great from a visual perspective with each author displaying under their own self-titled category, and also wouldn't work from an organizational perspective as those authors are not categories themselves but rather entities who fall under one or more categories such as, well, authors
This would work, however I (perhaps stubbornly) believe that if a tag inherits from a tag marked as a category then it should belong under that category. If a case such as the one you described with the genres and authors arise, I would consider that an issue with the tagging approach rather than the category system itself. Same if there was an instance where a separate category didn't make sense for a group of tags - in that case, just don't display that tag as a category and instead rethink the nature of how you would like certain tags to be organized. I'm open to hearing more cases against this if they arise, though. Life is full of more edge cases than I can ever encounter just by my own experiences, after all.
This would also unfortunately undermine much of the automatic inheritance provided by the tag category system. For example I have several tags such as "Media Type" for photos vs illustrations which I keep as a child of "Meta Tags". This allows me to have |
Thanks for the detailed reply! I understand that these issues arise from the way I'm treating these tag hierarchies, I think the described component tags would solve my problem, because essentially I want to bundle an assortment of tags under a new name, so I don't have to assign all of the bundled tags separately and manually to an entry. Perhaps a more clear example would be something like a |
At least in the current state of the branch, I have found that the tag that is marked as the category does not show up under its own category when assigned to an entry, they display under |
I see, I just happened to see and fix that bug (88b886c) after your message haha. It slipped past me while I was working on this latest approach to recursive categories, but it should be fixed now where category tags themselves show under their own categories. |
This... might a good example of an issue with the category system. Another one I realized could be having a "Movie" tag and a "Character" tag, with a tag "Shrek" (Character) inheriting from "Shrek" (Movie) and "Character". If "Character" is marked as a category, all is fine. But if you also wanted "Movie" as a category, then "Shrek" (Character) would also show up as a "Movie" because it is a Shrek Movie franchise entity. Not ideal. But that also comes with a couple quirks of the tagging method:
I'll continue to give this some thought in the hopes that there's a simple and general solution here either in tagging methods themselves, with 9.6 component tags, or simply with a tweak to how tag categories operate. My worry is that there won't be a way to fix this without doing something much more tailored and involved to category tags, such as having an extra subtag box where users can list which potential child tags this category should not show. That may function but I would consider that a very messy fix that would also have to extend as an option across all types of tags. Of course in the meantime, I'm very open to other proposals or scenarios that may help shed some more light on this. |
ecaca29
to
efb71ca
Compare
@Computerdores I've reworked the migration of edited built-in JSON tags in a way that addresses all of the comments regarding the For the rest of the comments on
And as far as this goes, I agree that it's not ideal but I certainly wasn't expecting to have to refactor the item grid selection system to behave more like how it did in v9.4 as a part of this feature (whose purpose was to add a future v9.6 feature in order to avoid bugs stemming from flaws in the how fields are connected to entries in the DB). So I guess I'd consider rewriting all of those tests to be out of the scope of this PR, especially since they'll likely all need to be rewritten again at some point. In my opinion it was just too early for those tests to exist. |
I've just made a reproduction with some stock videos. I've added some tags to them to help simulate my real library, but I doubt that matters?
I close the old version, reopen with a build from this PR (haven't pulled new changes since our last conversation though) I just pulled In this test I am selecting a single file. Just for completeness I'm going to try to see if this problem also happens on a fresh library with this build "Show in File Explorer" does not work from the rightclick menu on the entry either, but both options work from rightclick on the preview window |
I've just replicated this, however it's not limited to just videos - the "open file" right-click options just no longer work in the thumbnail grid for me. That's definitely an area I touched in this PR, so I'll give it a look. Thank you for your thorough response!
|
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.
Has my approval as all comments have been addressed!
I did just notice that the tests are failing, do you know what is up with that? |
Mentioned here: #655 (comment)
|
I have narrowed down the problem to this line: https://github.com/TagStudioDev/TagStudio/blob/tag-categories-feat/tagstudio/src/core/library/alchemy/library.py#L897 |
I managed to "fix" it by grabbing the built-in tags from the DB, editing the color, and then re-adding/"updating" the modified tags to the DB. Why this works, and why I couldn't do it in one step, I don't know... # Tags
for tag in json_lib.tags:
self.add_tag(
Tag(
id=tag.id,
name=tag.name,
shorthand=tag.shorthand,
color=TagColor.get_color_from_str(tag.color),
)
)
# Apply user edits to built-in JSON tags.
if tag.id in range(RESERVED_TAG_START, RESERVED_TAG_END + 1):
updated_tag = self.get_tag(tag.id)
updated_tag.color = TagColor.get_color_from_str(tag.color)
self.update_tag(updated_tag) # NOTE: This just calls add_tag? |
I have the strong suspicion that it is ORM shenanigans. Probably something like: The ORM notices that the Tag object that is being added is already being tracked by it and so instead of being INSERTed it gets UPDATEd instead |
Thank you SO much for the review, @Computerdores and @Tishj!! I know this was a massive one to look over so I really appreciate it! |
Thanks for your continued work on TagStudio, it's great to see the strides that the project is making! 💪 |
Replaces "Tag Fields" with Tag Categories.
Instead of tags needing to be added to a Tag field type such as "Meta Tags", "Content Tags", or simply the "Tags" field, tags are now added directly to file entries with no intermediary step. While tag field types offered a method to further organize tags, it was cumbersome, inflexible, and simply not fully fleshed out. Tag Categories offer all of the previous (intentional) functionality while greatly increasing the ease of use and customization.
How it works
Tags now have an additional boolean property that states whether or not a tag is marked as a "category" or not. Tags marked as categories have no functional difference in the backend but are treated specially by frontend views. Category tags take on the appearance similar to the previous tag fields while automatically showing all tags inheriting from that tag underneath its own group. This means that duplicates of tags can appear on entries if they inherit from multiple categories, however this is by design. Any tags not inheriting from a category tag will simply show under a default "Tag" field-like section.
The previous built-in tags, "Favorite" and "Archived", now inherit from a new built-in "Meta Tags" tag that's marked as a category by default. This means that the appearance of these tags is effectively unchanged from how they're added via the badge buttons - however the organization is now fully automatic, universal across all tag adding sources, and fully customizable by the user.
This does mean that existing user tag field configurations will be changed. Instead of tags being organized into field types on an per-entry basis, tags themselves determine their organizational layout via the new category flag. Any tags (not currently inheriting from either the "Favorite" or "Archived" tags) will be shown under the default "Tags" header upon migrating from v9.4 libraries. If the user wishes to permanently mark some of their own tags as "Meta Tags" they can easily do that by having these tags inherit from the new built-in "Meta Tags" tag. And if they'd rather set some of their own existing tags as categories (i.e. "People", "Places", "Characters") then they are free to do so as well!
Related Changes
A major component of this PR has been a comprehensive refactor of the preview panel. The panel has been broken up in the following ways:
PreviewPanel
has been made a lightweight container widgetPreviewThumb
FileAttributes
FieldContainers
The following features have not been re-implemented for the following reasons:
As a bonus with this approach, this PR fixes numerous issues present with field editing and rendering, both documented and undocumented since the SQL switch. Currently this PR is set to close #92, close #249, close #18, close #321, and close #416.