Skip to content
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

TaggableManager.set() API does not match RelatedManager.set() #686

Closed
rjmackay opened this issue Aug 20, 2020 · 5 comments
Closed

TaggableManager.set() API does not match RelatedManager.set() #686

rjmackay opened this issue Aug 20, 2020 · 5 comments

Comments

@rjmackay
Copy link

TaggableManager has set(*tags, clear=False) while RelatedManager expects the tag objects as a list ie set(tags, bulk=True, clear=False, through_defaults=None).
Would it be possible to support setting tags as a list/tuple too? The different interface makes it harder to plug that TaggableManager into other django libraries.

@auvipy
Copy link
Contributor

auvipy commented Aug 21, 2020

feel free to dig in and start a WIP PR to improve it?

@rtpg
Copy link
Contributor

rtpg commented Apr 14, 2021

I'm tempted to qualify this as a bug, due to not following SOLID. Thanks for pointing this out. A simple base case here would be to take params, but throw a NotImplementedError on stuff we don't support.

@coredumperror
Copy link

I agree 100% that this is a bug. I've now been burned by this difference in APIs between TaggableManager.set() and RelatedManager.set(), as I'm trying to use the same code to set various different types of tags on objects, and only one of those types is handled by django-taggit. So the API being different screws me.

@rtpg
Copy link
Contributor

rtpg commented Sep 9, 2021

@coredumperror feel free to send in a PR, I'll gladly review it to get it in. I think when I made the original comment I didn't see a straightforward way of getting it to work

@pablolmedorado
Copy link
Contributor

pablolmedorado commented Nov 7, 2021

I've been facing this issue trying to use a "tags" field together with the library "django-import-export". It's been impossible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants