Opened 7 years ago

Closed 7 years ago

#24927 closed Cleanup/optimization (fixed)

django.utils.six and django.utils.encoding both define python_2_unicode_compatible

Reported by: Marten Kenbeek Owned by: Markus Holtermann
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: me@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

django.utils.six and django.utils.encoding define the exact same method definition of python_2_unicode_compatible, which is not very DRY. It might be a good idea to replace one of them with a convenience import of the other.

Change History (10)

comment:1 Changed 7 years ago by Matthew Somerville

Has patch: set

The function was added to six in 1.9.0, copied from Django's code. PR at https://github.com/django/django/pull/4784 to have it include one from the other, I was unsure whether it was better to maintain the function in encoding.

comment:2 Changed 7 years ago by Andriy Sokolovskiy

Cc: me@… added
Easy pickings: unset

If we're going to use the method from six, seems we should to deprecate import from django utils, and replace imports inside the django code to use six import instead of utils.

comment:3 Changed 7 years ago by Carl Meyer

I don't think it's worth putting users through a deprecation process on the import location of python_2_unicode_compatible, when we could just provide a non-deprecated import shim and leave it there until Python 2 support is dropped entirely. That's a lot of hassle for Django users in order to save us a single import line in django.utils.encoding.

I do think it's good to switch to relying on the implementation in six.

comment:4 Changed 7 years ago by Markus Holtermann

Owner: changed from nobody to Markus Holtermann
Status: newassigned

comment:5 Changed 7 years ago by Markus Holtermann

Triage Stage: UnreviewedAccepted

comment:6 Changed 7 years ago by Carl Meyer

@markush there was already a PR contributed, see above

comment:7 Changed 7 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:8 Changed 7 years ago by Markus Holtermann

Oh, missed that. Thanks Carl.

comment:9 Changed 7 years ago by Aymeric Augustin

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I agree, let's drop our version and import six'. As Carl said on the PR a comment explaining the import is needed.

comment:10 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In c91bc68:

Fixed #24927 -- Used python_2_unicode_compatible from six

Note: See TracTickets for help on using tickets.
Back to Top