search

yourlabs / django-autocomplete-light

A fresh approach to autocomplete implementations, specially for Django. Status: v4 alpha, v3 stable, v2 & v1 deprecated.
https://django-autocomplete-light.readthedocs.io
MIT License
1.81k stars 468 forks source link

Remove six dependency #1294

Closed claudep closed 1 year ago

adamchainz commented 2 years ago

LGTM!

jpic commented 2 years ago

I believe DAL still has users on Python 2, it's not tested but the policy is to accept whatever PR that doesn't break the test suite on python3

claudep commented 2 years ago

I guess Python 2 users should all have pinned dependencies these days, so they can still use an already released DAL.

jpic commented 2 years ago

If I'm understanding the roadmap correctly, we can merge this when we release Django 4 support https://django-autocomplete-light.readthedocs.io/en/master/#roadmap

adamchainz commented 1 year ago

Why was this closed?

jpic commented 1 year ago

Wrong execution of my plan to automate releases from github actions: don't do a commit --amend in there :facepalm:

jpic commented 1 year ago

Nonetheless, as explained in the comments, we have a policy of leaving older python/django support code as long as it's not hurting support for current releases, we don't maintain it officially but we do merge community contributions to support legacy versions when it shows up, so we're probably not going to merge it at all anyway.

But I need to rewrite what broke the PRs though so I'll be working on that.

claudep commented 1 year ago

I have some sympathy for this policy, however in this case you are forcing projects to install a possibly unwanted old dependency, which only uses is to support Python 2, which is unsupported by your package. Removing this compatibility layer will also (slightly) improve the performance of your package.

jpic commented 1 year ago

I'm not forcing anyone to install django-autocomplete-light, however, I am trying not to break it for users who have deployed it.

I'm compiling a Python 2 so that I can verify if this package still works with Python 2 to some extent, in which case, I'm not going to let this pull request break god knows how many projects and take away users freedom to use it with unsupported versions just for the sake of it based on an un-measured and probably insignificant performance improvement, as six itself is still liwghtweight and actively maintained.

Since you're both Django contributors, let me remind you: this app shouldn't exist, it only exists because browsers break when Django users render a model form with a foreign key on a table that has so many entries that the browser can't handle it and nobody ever cared enough to face it and would rather let the users hang out dry, nor even making workarounds easier for users which Django leaves us no choice but to monkey patch: https://django-autocomplete-light.readthedocs.io/en/master/tutorial.html#automation-with-djhacker

Anyway, select2 will also, like Python 2, enter community support as well and I'm guessing all users will be happy that we don't just drop the code that we don't want to support anymore and that we let live the community for years while we go ahead and release autocomplete-light web component as main supported solution because that still next on the roadmap, and breaking existing deployments / the reality of users who don't have the skills or the manpower to migrate their code just yet just for the sake of theoretical considerations, is not part of that roadmap.

jpic commented 1 year ago

Turns out Python 2 support was contributed by a user in 2019, and broken by another in 2020, and nobody has complained since them, so I am merging this.

Sorry for breaking the MR and thanks for your contribution :tophat: