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

Python3 compatibility, fulltext tag search and document update #102

Closed
wants to merge 18 commits into from

Conversation

dany-nonstop
Copy link

@dany-nonstop dany-nonstop commented Oct 30, 2020

Changes made

  • added compatibility with python3, change all use of django.utils.six (deprecated) to six
  • added tag option autocomplete_fulltext to make full text search on autocompletion, default is off
  • updated document on default setting, especially when jQuery is loaded in the other part of the project
  • updated document on autocomplete_fulltext
  • fix "division by 0" when calling weight() on a list of new tags imported without any count (everything is 0)

they may break tagulous on python2, never tested.

@dany-nonstop
Copy link
Author

@radiac I like the concise tag tree implementation in tagulous and have used it in my python3 + django3 project. While working on my project I have added these changes to make it work. Thanks for making it available to the community.

@coveralls
Copy link

coveralls commented Oct 30, 2020

Coverage Status

Coverage increased (+0.003%) to 95.203% when pulling 9efab26 on dany-nonstop:develop into b73b6c9 on radiac:develop.

calling weight() would result in "division by 0" error as everything is zero
@dany-nonstop
Copy link
Author

@radiac whenever you're available, could you please check the code and merge them to your current branch? thanks.

@dany-nonstop
Copy link
Author

Hi, @radiac how can I contribute my code to your project? Thanks!

@BoPeng
Copy link
Contributor

BoPeng commented Feb 24, 2021

I was looking for a similar features but I simply used a customized autocomplete_view function. Perhaps this is why @radiac did not want to add this as one of the core features?

@dany-nonstop
Copy link
Author

Hi, @BoPeng your comment is highly appreciated, thanks, I would say it's a possible solution. But I don't know why @radiac didn't even care to comment before closing this PR.

@BoPeng
Copy link
Contributor

BoPeng commented Mar 10, 2021

I have a PR pending for more flexible view function. It is much simpler than your solution but might still not be merged. I am using a fork for my project anyway.

@radiac
Copy link
Owner

radiac commented Mar 10, 2021

Sorry for the delay - I haven't closed the pr, it looks interesting, but unfortunately it came in while I was finishing up the refactor to remove py2, so it wasn't a simple one to merge - I'll need to cherry pick bits. The past few months have been very busy, but I hope to get to it soon.

radiac added a commit that referenced this pull request Aug 6, 2021
radiac added a commit that referenced this pull request Aug 6, 2021
When there are tags but all have a zero count, the max count should
be 1 to avoid unexpected results after division by zero
@radiac radiac mentioned this pull request Aug 6, 2021
@radiac
Copy link
Owner

radiac commented Aug 7, 2021

Thanks for the PR, finally had a chance to pick out the compatible changes and add some tests. Closing, merged via #128.

@radiac radiac closed this Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants